Correct profile source validation and identity

internal/profile/repository_test.go

@@ -2,8 +2,8 @@ package profile
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -126,63 +126,6 @@ temperature: 0.1
 		}
 	})
 
-	t.Run("valid profile with JSON-compatible extra params", func(t *testing.T) {
-		writeProfileTestFile(t, filepath.Join(tmpDir, "json-extra-params.yaml"), `
-id: json-extra-params
-endpoint: http://localhost:8000/v1
-model: nested-model
-extra_params:
-  string_value: enabled
-  number_value: 42
-  boolean_value: true
-  object_value:
-    nested: value
-    count: 2
-  array_value:
-    - first
-    - 3
-    - false
-`)
-
-		p, err := repo.GetProfile(ctx, "json-extra-params")
-		if err != nil {
-			t.Fatalf("expected no error, got %v", err)
-		}
-
-		var got map[string]any
-		encoded, err := json.Marshal(p.ExtraParams)
-		if err != nil {
-			t.Fatalf("expected extra_params to marshal as JSON, got %v", err)
-		}
-		if err := json.Unmarshal(encoded, &got); err != nil {
-			t.Fatalf("expected extra_params JSON to decode, got %v", err)
-		}
-
-		if got["string_value"] != "enabled" {
-			t.Fatalf("unexpected string extra param: %#v", got["string_value"])
-		}
-		if got["number_value"] != float64(42) {
-			t.Fatalf("unexpected number extra param: %#v", got["number_value"])
-		}
-		if got["boolean_value"] != true {
-			t.Fatalf("unexpected boolean extra param: %#v", got["boolean_value"])
-		}
-		objectValue, ok := got["object_value"].(map[string]any)
-		if !ok {
-			t.Fatalf("expected object extra param, got %#v", got["object_value"])
-		}
-		if objectValue["nested"] != "value" || objectValue["count"] != float64(2) {
-			t.Fatalf("unexpected object extra param: %#v", objectValue)
-		}
-		arrayValue, ok := got["array_value"].([]any)
-		if !ok {
-			t.Fatalf("expected array extra param, got %#v", got["array_value"])
-		}
-		if len(arrayValue) != 3 || arrayValue[0] != "first" || arrayValue[1] != float64(3) || arrayValue[2] != false {
-			t.Fatalf("unexpected array extra param: %#v", arrayValue)
-		}
-	})
-
 	t.Run("duplicate profile IDs fail as ambiguous", func(t *testing.T) {
 		writeProfileTestFile(t, filepath.Join(tmpDir, "duplicate-profile-a.yaml"), `
 id: duplicate-profile
@@ -245,10 +188,10 @@ api_key: secret
 		}
 	})
 
-	t.Run("invalid yaml", func(t *testing.T) {
+	t.Run("unidentifiable invalid yaml is unrelated", func(t *testing.T) {
 		_, err := repo.GetProfile(ctx, "invalid_yaml")
-		if !errors.Is(err, ErrInvalidYAML) {
-			t.Fatalf("expected ErrInvalidYAML, got %v", err)
+		if !errors.Is(err, ErrProfileNotFound) {
+			t.Fatalf("expected ErrProfileNotFound, got %v", err)
 		}
 	})
 
@@ -274,14 +217,14 @@ api_key: secret
 	})
 
 	t.Run("unknown field", func(t *testing.T) {
-		_, err := repo.GetProfile(ctx, "unknown_field")
+		_, err := repo.GetProfile(ctx, "unknown-field")
 		if !errors.Is(err, ErrInvalidYAML) {
 			t.Fatalf("expected ErrInvalidYAML for strict decode unknown field, got %v", err)
 		}
 	})
 
 	t.Run("raw api_key rejected", func(t *testing.T) {
-		_, err := repo.GetProfile(ctx, "raw_api_key")
+		_, err := repo.GetProfile(ctx, "raw-api-key")
 		if !errors.Is(err, ErrRawAPIKeyNotAllowed) {
 			t.Fatalf("expected ErrRawAPIKeyNotAllowed, got %v", err)
 		}
@@ -406,6 +349,377 @@ model: second
 	})
 }
 
+func TestProfileRepositoriesValidateExtraParams(t *testing.T) {
+	const validProfile = `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: model
+extra_params:
+  string_value: enabled
+  object_value:
+    nested: true
+  array_value:
+    - first
+    - 3
+`
+	tests := []struct {
+		name        string
+		definition  string
+		wantErr     bool
+		diagnostics []string
+	}{
+		{name: "valid nested values", definition: validProfile},
+		{
+			name: "empty key",
+			definition: `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: model
+extra_params:
+  "": value
+`,
+			wantErr:     true,
+			diagnostics: []string{"extra_params", "key must not be empty"},
+		},
+		{
+			name: "non-finite value",
+			definition: `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: model
+extra_params:
+  invalid: .nan
+`,
+			wantErr:     true,
+			diagnostics: []string{"extra_params.invalid", "must be finite"},
+		},
+		{
+			name: "nested non-finite value",
+			definition: `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: model
+extra_params:
+  outer:
+    invalid: .inf
+`,
+			wantErr:     true,
+			diagnostics: []string{"extra_params.outer.invalid", "must be finite"},
+		},
+		{
+			name: "unsupported decoded value",
+			definition: `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: model
+extra_params:
+  timestamp: 2026-08-11T12:34:56Z
+`,
+			wantErr:     true,
+			diagnostics: []string{"extra_params.timestamp", "unsupported JSON value type"},
+		},
+		{
+			name:        "excessive nesting",
+			definition:  deeplyNestedExtraParamsProfile(101),
+			wantErr:     true,
+			diagnostics: []string{"extra_params", "JSON container depth limit exceeded"},
+		},
+	}
+
+	for _, source := range profileRepositorySources() {
+		for _, tc := range tests {
+			t.Run(source.name+"/"+tc.name, func(t *testing.T) {
+				repo := source.newRepository(t, map[string]string{"selected.yaml": tc.definition})
+				got, err := repo.GetProfile(context.Background(), "selected-profile")
+				if tc.wantErr {
+					if !errors.Is(err, ErrInvalidProfile) {
+						t.Fatalf("expected ErrInvalidProfile, got %v", err)
+					}
+					if !strings.Contains(err.Error(), "selected.yaml") {
+						t.Fatalf("expected source path in error, got %v", err)
+					}
+					for _, diagnostic := range tc.diagnostics {
+						if !strings.Contains(err.Error(), diagnostic) {
+							t.Fatalf("expected error to contain %q, got %v", diagnostic, err)
+						}
+					}
+					return
+				}
+				if err != nil {
+					t.Fatalf("load valid profile: %v", err)
+				}
+				if got.ExtraParams["string_value"] != "enabled" {
+					t.Fatalf("unexpected copied extra params: %#v", got.ExtraParams)
+				}
+				objectValue, objectOK := got.ExtraParams["object_value"].(map[string]any)
+				arrayValue, arrayOK := got.ExtraParams["array_value"].([]any)
+				if !objectOK || objectValue["nested"] != true ||
+					!arrayOK || len(arrayValue) != 2 || arrayValue[0] != "first" || arrayValue[1] != 3 {
+					t.Fatalf("unexpected copied nested extra params: %#v", got.ExtraParams)
+				}
+			})
+		}
+	}
+}
+
+func TestProfileRepositoriesSelectCanonicalYAMLID(t *testing.T) {
+	const validProfile = `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: selected-model
+`
+	tests := []struct {
+		name        string
+		files       map[string]string
+		wantErr     error
+		diagnostics []string
+	}{
+		{
+			name: "same stem unknown field with different id is unrelated",
+			files: map[string]string{
+				"selected-profile.yaml": `
+id: unrelated-profile
+endpoint: http://localhost:8000/v1
+model: unrelated
+unknown: true
+`,
+				"valid.yaml": validProfile,
+			},
+		},
+		{
+			name: "same stem unidentifiable yaml is unrelated",
+			files: map[string]string{
+				"selected-profile.yaml": "id: [",
+				"valid.yaml":            validProfile,
+			},
+		},
+		{
+			name: "same stem raw key with different id is unrelated",
+			files: map[string]string{
+				"selected-profile.yaml": `
+id: unrelated-profile
+endpoint: http://localhost:8000/v1
+model: unrelated
+api_key: secret
+`,
+				"valid.yaml": validProfile,
+			},
+		},
+		{
+			name: "leading and trailing whitespace is normalized",
+			files: map[string]string{
+				"padded.yaml": `
+id: "  selected-profile  "
+endpoint: http://localhost:8000/v1
+model: selected-model
+`,
+			},
+		},
+		{
+			name: "blank id is unrelated",
+			files: map[string]string{
+				"selected-profile.yaml": `
+id: "   "
+endpoint: http://localhost:8000/v1
+model: unrelated
+`,
+			},
+			wantErr: ErrProfileNotFound,
+		},
+		{
+			name: "normalized duplicates are ambiguous",
+			files: map[string]string{
+				"first.yaml": validProfile,
+				"nested/second.yaml": `
+id: " selected-profile "
+endpoint: http://localhost:8000/v1
+model: duplicate
+`,
+			},
+			wantErr:     ErrInvalidProfile,
+			diagnostics: []string{"duplicate execution profile id", "first.yaml", "nested/second.yaml"},
+		},
+		{
+			name: "selected unknown field is authoritative",
+			files: map[string]string{
+				"malformed.yaml": `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: selected-model
+unknown: true
+`,
+			},
+			wantErr:     ErrInvalidYAML,
+			diagnostics: []string{"malformed.yaml"},
+		},
+		{
+			name: "selected raw key is authoritative",
+			files: map[string]string{
+				"insecure.yaml": `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: selected-model
+api_key: secret
+`,
+			},
+			wantErr:     ErrRawAPIKeyNotAllowed,
+			diagnostics: []string{"insecure.yaml"},
+		},
+		{
+			name: "selected identity in an additional document is authoritative",
+			files: map[string]string{
+				"additional-document.yaml": `
+---
+---
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: selected-model
+`,
+			},
+			wantErr:     ErrInvalidYAML,
+			diagnostics: []string{"additional-document.yaml", "exactly one YAML document"},
+		},
+	}
+
+	for _, source := range profileRepositorySources() {
+		for _, tc := range tests {
+			t.Run(source.name+"/"+tc.name, func(t *testing.T) {
+				repo := source.newRepository(t, tc.files)
+				got, err := repo.GetProfile(context.Background(), " selected-profile ")
+				if tc.wantErr != nil {
+					if !errors.Is(err, tc.wantErr) {
+						t.Fatalf("expected %v, got %v", tc.wantErr, err)
+					}
+					for _, diagnostic := range tc.diagnostics {
+						if !strings.Contains(err.Error(), diagnostic) {
+							t.Fatalf("expected error to contain %q, got %v", diagnostic, err)
+						}
+					}
+					return
+				}
+				if err != nil {
+					t.Fatalf("load selected profile: %v", err)
+				}
+				if got.ID != "selected-profile" || got.Model != "selected-model" {
+					t.Fatalf("unexpected selected profile: %+v", got)
+				}
+			})
+		}
+	}
+}
+
+func TestProfileRepositoriesRequireOneYAMLDocument(t *testing.T) {
+	const profile = `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: selected-model
+`
+	tests := []struct {
+		name    string
+		suffix  string
+		wantErr bool
+	}{
+		{name: "comments and trailing whitespace", suffix: "\n# trailing comment\n\n"},
+		{name: "second populated document", suffix: "\n---\nid: another\n", wantErr: true},
+		{name: "second empty document", suffix: "\n---\n", wantErr: true},
+		{name: "malformed trailing yaml", suffix: "\n---\n[", wantErr: true},
+		{name: "raw key in trailing document", suffix: "\n---\napi_key: secret\n", wantErr: true},
+	}
+
+	for _, source := range profileRepositorySources() {
+		for _, tc := range tests {
+			t.Run(source.name+"/"+tc.name, func(t *testing.T) {
+				repo := source.newRepository(t, map[string]string{"definition.yaml": profile + tc.suffix})
+				got, err := repo.GetProfile(context.Background(), "selected-profile")
+				if tc.wantErr {
+					if !errors.Is(err, ErrInvalidYAML) {
+						t.Fatalf("expected ErrInvalidYAML, got %v", err)
+					}
+					if !strings.Contains(err.Error(), "definition.yaml") {
+						t.Fatalf("expected source path in error, got %v", err)
+					}
+					return
+				}
+				if err != nil {
+					t.Fatalf("load one-document profile: %v", err)
+				}
+				if got.ID != "selected-profile" {
+					t.Fatalf("unexpected profile: %+v", got)
+				}
+			})
+		}
+	}
+}
+
+func TestProfileRepositoriesPreserveOverlayFallbackRules(t *testing.T) {
+	fallback := staticProfileRepo{profiles: map[string]*domain.ExecutionProfile{
+		"selected-profile": {ID: "selected-profile", Endpoint: "http://fallback", Model: "fallback-model"},
+	}}
+	tests := []struct {
+		name      string
+		files     map[string]string
+		wantModel string
+		wantErr   error
+	}{
+		{
+			name: "same stem malformed different id falls back",
+			files: map[string]string{
+				"selected-profile.yaml": `
+id: unrelated-profile
+endpoint: http://localhost:8000/v1
+model: unrelated
+unknown: true
+`,
+			},
+			wantModel: "fallback-model",
+		},
+		{
+			name: "blank id falls back",
+			files: map[string]string{
+				"selected-profile.yaml": `
+id: "   "
+endpoint: http://localhost:8000/v1
+model: unrelated
+`,
+			},
+			wantModel: "fallback-model",
+		},
+		{
+			name: "selected malformed profile stops fallback",
+			files: map[string]string{
+				"other-name.yaml": `
+id: selected-profile
+endpoint: http://localhost:8000/v1
+model: selected
+unknown: true
+`,
+			},
+			wantErr: ErrInvalidYAML,
+		},
+	}
+
+	for _, source := range profileRepositorySources() {
+		for _, tc := range tests {
+			t.Run(source.name+"/"+tc.name, func(t *testing.T) {
+				primary := source.newRepository(t, tc.files)
+				got, err := NewOverlayRepository(primary, fallback).GetProfile(context.Background(), "selected-profile")
+				if tc.wantErr != nil {
+					if !errors.Is(err, tc.wantErr) {
+						t.Fatalf("expected %v, got %v", tc.wantErr, err)
+					}
+					return
+				}
+				if err != nil {
+					t.Fatalf("load fallback profile: %v", err)
+				}
+				if got.Model != tc.wantModel {
+					t.Fatalf("model = %q, want %q", got.Model, tc.wantModel)
+				}
+			})
+		}
+	}
+}
+
 func TestProfileRepositoriesRejectInvalidExecutionSettings(t *testing.T) {
 	ctx := context.Background()
 
@@ -530,6 +844,52 @@ func TestOverlayRepository(t *testing.T) {
 	})
 }
 
+type profileRepositorySource struct {
+	name          string
+	newRepository func(t *testing.T, files map[string]string) Repository
+}
+
+func profileRepositorySources() []profileRepositorySource {
+	return []profileRepositorySource{
+		{
+			name: "operating system",
+			newRepository: func(t *testing.T, files map[string]string) Repository {
+				t.Helper()
+				root := t.TempDir()
+				for name, content := range files {
+					filePath := filepath.Join(root, filepath.FromSlash(name))
+					if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil {
+						t.Fatalf("create profile directory: %v", err)
+					}
+					writeProfileTestFile(t, filePath, content)
+				}
+				return NewFilesystemRepository(root)
+			},
+		},
+		{
+			name: "filesystem",
+			newRepository: func(t *testing.T, files map[string]string) Repository {
+				t.Helper()
+				fsys := make(fstest.MapFS, len(files))
+				for name, content := range files {
+					fsys[name] = profileMapFile(content)
+				}
+				return NewFSRepository(fsys, ".")
+			},
+		},
+	}
+}
+
+func deeplyNestedExtraParamsProfile(depth int) string {
+	var definition strings.Builder
+	definition.WriteString("id: selected-profile\nendpoint: http://localhost:8000/v1\nmodel: model\nextra_params:\n")
+	for level := 0; level < depth; level++ {
+		fmt.Fprintf(&definition, "%slevel_%d:\n", strings.Repeat("  ", level+1), level)
+	}
+	fmt.Fprintf(&definition, "%svalue: true\n", strings.Repeat("  ", depth+1))
+	return definition.String()
+}
+
 func profileMapFile(content string) *fstest.MapFile {
 	return &fstest.MapFile{Data: []byte(strings.TrimLeft(content, "\n"))}
 }