From 70e0ea0cf0fadd71ef4b21b6123d242028025a0a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 22:28:48 +0000 Subject: [PATCH] Correct profile source validation and identity --- docs/formats.md | 10 +- docs/internal/sources.md | 23 +- internal/filecatalog/catalog.go | 7 - internal/filecatalog/catalog_test.go | 20 - internal/profile/filesystem_repository.go | 102 ++++- internal/profile/repository_test.go | 486 +++++++++++++++++++--- public_contract_test.go | 37 +- 7 files changed, 566 insertions(+), 119 deletions(-) diff --git a/docs/formats.md b/docs/formats.md index 6fb1ba3..3575041 100644 --- a/docs/formats.md +++ b/docs/formats.md @@ -9,10 +9,10 @@ explains how to select these sources and invoke the engine. The owns the resulting outbound wire behavior. Prompt and profile sources recursively discover files ending in `.yaml` or -`.yml`. Each prompt-definition file contains exactly one YAML document; -comments and trailing whitespace are allowed. YAML decoding is strict: unknown -fields are errors for the selected definition. Definitions are selected by -their YAML `id`, not their file name or directory. +`.yml`. Each prompt-definition and profile file contains exactly one YAML +document; comments and trailing whitespace are allowed. YAML decoding is +strict: unknown fields are errors for the selected definition. Definitions are +selected by their YAML `id`, not their file name or directory. ## Prompt Definitions @@ -164,7 +164,7 @@ extra_params: | Field | Required | Meaning | | --- | --- | --- | -| `id` | yes | Non-empty profile identifier. IDs must be unique within one source. | +| `id` | yes | Profile identifier, trimmed before selection and publication. It must be non-empty after trimming and unique within one source after normalization. | | `backend` | unless `endpoint` is present | Backend registry ID. It is trimmed and registry membership is checked when the profile is prepared or inspected. | | `endpoint` | unless `backend` is present | Non-empty OpenAI-compatible base URL, including an API version path when required. When both connection fields are present, this overrides the backend endpoint without changing backend identity. | | `model` | yes | Non-empty provider model name. | diff --git a/docs/internal/sources.md b/docs/internal/sources.md index a10ec60..9660e69 100644 --- a/docs/internal/sources.md +++ b/docs/internal/sources.md @@ -40,13 +40,22 @@ duplicate detection, and source containment: ## Profiles And Built-Ins `internal/profile` loads and validates execution profiles from an -operating-system filesystem or an `fs.FS`. Its overlay repository consults the -next repository only when the higher-precedence repository reports that a -profile is absent. Strict YAML decoding recognizes the optional `backend` -field, trims its value, and requires a model plus at least one non-blank -backend or endpoint. Loading does not check registry membership because the -available registry belongs to the assembled engine; the runner checks -membership during preparation and exact profile inspection. +operating-system filesystem or an `fs.FS`. A file contains exactly one YAML +document and its trimmed YAML `id` is its only selection identity; filenames do +not confer authority. Strict selected decoding recognizes the optional +`backend` field, trims its value, and requires a model plus at least one +non-blank backend or endpoint. File-backed `extra_params` values are validated +and defensively copied through the shared bounded JSON-value owner before a +profile is published. OpenAI-compatible reserved-field policy remains with the +model-client and backend-registry owners. + +The overlay repository consults the next repository only when the +higher-precedence repository reports that a profile is absent. A reliably +selected malformed profile stops fallback, while an unrelated malformed file +does not become authoritative through its filename. Loading does not check +backend registry membership because the available registry belongs to the +assembled engine; the runner checks membership during preparation and exact +profile inspection. The root engine assembles profile repositories in precedence order: in-memory profiles, one ordinary configured source, an application fallback source, then diff --git a/internal/filecatalog/catalog.go b/internal/filecatalog/catalog.go index 162afdc..dcc148d 100644 --- a/internal/filecatalog/catalog.go +++ b/internal/filecatalog/catalog.go @@ -134,13 +134,6 @@ func containsFSPath(root string, name string) bool { return name == root || strings.HasPrefix(name, strings.TrimSuffix(root, "/")+"/") } -// Stem strips .yaml or .yml from a file name. -func Stem(name string) string { - name = strings.TrimSuffix(name, ".yaml") - name = strings.TrimSuffix(name, ".yml") - return name -} - func IsYAMLFile(name string) bool { return strings.HasSuffix(name, ".yaml") || strings.HasSuffix(name, ".yml") } diff --git a/internal/filecatalog/catalog_test.go b/internal/filecatalog/catalog_test.go index 90f9a6a..171e7cd 100644 --- a/internal/filecatalog/catalog_test.go +++ b/internal/filecatalog/catalog_test.go @@ -237,26 +237,6 @@ func TestResolveFSPath(t *testing.T) { } } -func TestStemStripsYAMLExtensions(t *testing.T) { - tests := []struct { - name string - in string - want string - }{ - {name: "yaml", in: "prompt.yaml", want: "prompt"}, - {name: "yml", in: "profile.yml", want: "profile"}, - {name: "other", in: "file.txt", want: "file.txt"}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if got := Stem(tc.in); got != tc.want { - t.Fatalf("expected %q, got %q", tc.want, got) - } - }) - } -} - func TestIsYAMLFile(t *testing.T) { tests := []struct { name string diff --git a/internal/profile/filesystem_repository.go b/internal/profile/filesystem_repository.go index a186012..6187f09 100644 --- a/internal/profile/filesystem_repository.go +++ b/internal/profile/filesystem_repository.go @@ -5,13 +5,14 @@ import ( "context" "errors" "fmt" + "io" "io/fs" "os" - "path" "strings" "gitea.maximumdirect.net/eric/promptkit/internal/domain" "gitea.maximumdirect.net/eric/promptkit/internal/filecatalog" + "gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue" "gopkg.in/yaml.v3" ) @@ -73,7 +74,8 @@ func (r *overlayRepository) GetProfile(ctx context.Context, id string) (*domain. } func loadProfile(ctx context.Context, fsys fs.FS, root string, id string) (*domain.ExecutionProfile, error) { - if strings.TrimSpace(id) == "" { + id = strings.TrimSpace(id) + if id == "" { return nil, fmt.Errorf("%w: profile id is required", ErrInvalidProfile) } if fsys == nil { @@ -94,13 +96,18 @@ func loadProfile(ctx context.Context, fsys fs.FS, root string, id string) (*doma } relPath := filecatalog.DisplayPath(root, fullPath) - fileMatch := filecatalog.Stem(path.Base(fullPath)) == id data, err := fs.ReadFile(fsys, fullPath) if err != nil { return nil, fmt.Errorf("failed to read profile file %s: %w", relPath, err) } - metadata := readProfileFileMetadata(data) - idMatch := fileMatch || metadata.id == id + metadata, metadataErr := readProfileFileMetadata(data) + idMatch := metadata.matchesID(id) + if metadataErr != nil { + if idMatch { + return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, metadataErr) + } + continue + } if metadata.hasRawAPIKey { if idMatch { return nil, fmt.Errorf("%w: %s", ErrRawAPIKeyNotAllowed, relPath) @@ -108,28 +115,31 @@ func loadProfile(ctx context.Context, fsys fs.FS, root string, id string) (*doma continue } - var prof domain.ExecutionProfile - decoder := yaml.NewDecoder(bytes.NewReader(data)) - decoder.KnownFields(true) - if err := decoder.Decode(&prof); err != nil { + prof, err := decodeProfile(data) + if err != nil { if idMatch { return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, err) } continue } + prof.ID = strings.TrimSpace(prof.ID) if prof.ID != id { continue } prof.BackendID = strings.TrimSpace(prof.BackendID) - if err := validateProfile(&prof); err != nil { + prof.ExtraParams, err = jsonvalue.CopyMap(prof.ExtraParams) + if err != nil { + return nil, fmt.Errorf("%w: %s: %v", ErrInvalidProfile, relPath, err) + } + if err := validateProfile(prof); err != nil { if errors.Is(err, ErrRawAPIKeyNotAllowed) { return nil, fmt.Errorf("%w: %s", err, relPath) } return nil, fmt.Errorf("%w: %s: %v", ErrInvalidProfile, relPath, err) } matches = append(matches, profileMatch{ - profile: &prof, + profile: prof, path: relPath, }) } @@ -155,15 +165,36 @@ type profileMatch struct { } type profileFileMetadata struct { - id string + ids []string hasRawAPIKey bool } -func readProfileFileMetadata(data []byte) profileFileMetadata { +func readProfileFileMetadata(data []byte) (profileFileMetadata, error) { + decoder := yaml.NewDecoder(bytes.NewReader(data)) var node yaml.Node - if err := yaml.NewDecoder(bytes.NewReader(data)).Decode(&node); err != nil { - return profileFileMetadata{} + if err := decoder.Decode(&node); err != nil { + return profileFileMetadata{}, err } + metadata := profileMetadataFromNode(&node) + documentCount := 1 + for { + var trailing yaml.Node + err := decoder.Decode(&trailing) + if errors.Is(err, io.EOF) { + if documentCount == 1 { + return metadata, nil + } + return metadata, errors.New("profile file must contain exactly one YAML document") + } + if err != nil { + return metadata, err + } + documentCount++ + metadata.merge(profileMetadataFromNode(&trailing)) + } +} + +func profileMetadataFromNode(node *yaml.Node) profileFileMetadata { if node.Kind != yaml.DocumentNode || len(node.Content) == 0 { return profileFileMetadata{} } @@ -178,7 +209,7 @@ func readProfileFileMetadata(data []byte) profileFileMetadata { value := mapping.Content[i+1] switch key.Value { case "id": - metadata.id = strings.TrimSpace(value.Value) + metadata.ids = append(metadata.ids, strings.TrimSpace(value.Value)) case "api_key": metadata.hasRawAPIKey = true } @@ -186,6 +217,45 @@ func readProfileFileMetadata(data []byte) profileFileMetadata { return metadata } +func (m profileFileMetadata) matchesID(id string) bool { + for _, candidate := range m.ids { + if candidate == id { + return true + } + } + return false +} + +func (m *profileFileMetadata) merge(other profileFileMetadata) { + m.ids = append(m.ids, other.ids...) + m.hasRawAPIKey = m.hasRawAPIKey || other.hasRawAPIKey +} + +func decodeProfile(data []byte) (*domain.ExecutionProfile, error) { + var prof domain.ExecutionProfile + decoder := yaml.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(true) + if err := decoder.Decode(&prof); err != nil { + return nil, err + } + if err := requireYAMLStreamEnd(decoder); err != nil { + return nil, err + } + return &prof, nil +} + +func requireYAMLStreamEnd(decoder *yaml.Decoder) error { + var trailing yaml.Node + err := decoder.Decode(&trailing) + if errors.Is(err, io.EOF) { + return nil + } + if err != nil { + return err + } + return errors.New("profile file must contain exactly one YAML document") +} + func validateProfile(p *domain.ExecutionProfile) error { if strings.TrimSpace(p.ID) == "" { return errors.New("id is required") diff --git a/internal/profile/repository_test.go b/internal/profile/repository_test.go index 98523b6..f907761 100644 --- a/internal/profile/repository_test.go +++ b/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"))} } diff --git a/public_contract_test.go b/public_contract_test.go index f308db3..de095f2 100644 --- a/public_contract_test.go +++ b/public_contract_test.go @@ -77,6 +77,41 @@ api_key_env: PROMPTKIT_INSPECTION_ABSENT_KEY } } +func TestFileProfileNormalizedIDMatchesInspectionAndPreparation(t *testing.T) { + engine, err := promptkit.NewEngine(promptkit.Config{}, + promptkit.WithPromptFS(contractPromptFS("prompt", "normalized-profile", "message"), "."), + promptkit.WithProfileFS(fstest.MapFS{ + "profile.yaml": &fstest.MapFile{Data: []byte(` +id: " normalized-profile " +endpoint: http://profile.example/v1 +model: normalized-model +`)}, + }, "."), + ) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + + inspection, err := engine.InspectProfile(context.Background(), " normalized-profile ") + if err != nil { + t.Fatalf("inspect normalized profile: %v", err) + } + prepared, err := engine.Prepare(context.Background(), promptkit.RunRequest{ + PromptID: "prompt", + ProfileID: " normalized-profile ", + }) + if err != nil { + t.Fatalf("prepare with normalized profile: %v", err) + } + + if inspection.ProfileID != "normalized-profile" || + prepared.SelectedProfileID != inspection.ProfileID || + inspection.EffectiveModelParams.Model != "normalized-model" || + prepared.EffectiveModelParams.Model != inspection.EffectiveModelParams.Model { + t.Fatalf("inspection=%#v prepared=%#v", inspection, prepared) + } +} + func TestInspectProfilePreservesPublicErrorIdentities(t *testing.T) { newEngine := func(t *testing.T, options ...promptkit.Option) *promptkit.Engine { t.Helper() @@ -109,7 +144,7 @@ func TestInspectProfilePreservesPublicErrorIdentities(t *testing.T) { } malformed := newEngine(t, promptkit.WithProfileFS(fstest.MapFS{ - "broken.yaml": &fstest.MapFile{Data: []byte("id: broken\nendpoint: http://broken.example/v1\nmodel: model\nunknown: value\n")}, + "broken.yaml": &fstest.MapFile{Data: []byte("id: broken\nendpoint: http://broken.example/v1\nmodel: model\nextra_params:\n invalid: .nan\n")}, }, ".")) if result, err := malformed.InspectProfile(context.Background(), "broken"); result != nil || !errors.Is(err, promptkit.ErrProfileLoad) {