From 58ac3ce298a5ac42ae9a888fbd91780a0cd34e70 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 21:54:47 +0000 Subject: [PATCH] Correct engine construction edge cases --- engine.go | 15 ++- engine_test.go | 120 ++++++++++++++++++++++++ internal/defaults/defaults.go | 5 +- internal/filecatalog/catalog.go | 13 ++- internal/filecatalog/catalog_test.go | 7 +- internal/validate/standard_validator.go | 2 +- public_contract_test.go | 21 ++++- 7 files changed, 161 insertions(+), 22 deletions(-) diff --git a/engine.go b/engine.go index 0040ddc..6ee959f 100644 --- a/engine.go +++ b/engine.go @@ -461,21 +461,20 @@ func newProfileRepository(profileDir string, options engineOptions) profile.Repo } func fileSource(name string) (fs.FS, string, error) { - cleanName := strings.TrimSpace(name) - if cleanName == "" { + if strings.TrimSpace(name) == "" { return nil, "", ErrInvalidConfig } - dir := filepath.Dir(cleanName) - base := filepath.Base(cleanName) - if base == "." || base == string(filepath.Separator) || strings.TrimSpace(base) == "" { + dir := filepath.Dir(name) + base := filepath.Base(name) + if base == "." || base == string(filepath.Separator) { return nil, "", ErrInvalidConfig } - info, err := os.Stat(cleanName) + info, err := os.Stat(name) if err != nil { - return nil, "", fmt.Errorf("%w: failed to access source file %q: %v", ErrInvalidConfig, cleanName, err) + return nil, "", fmt.Errorf("%w: failed to access source file %q: %v", ErrInvalidConfig, name, err) } if info.IsDir() { - return nil, "", fmt.Errorf("%w: source path %q must be a file", ErrInvalidConfig, cleanName) + return nil, "", fmt.Errorf("%w: source path %q must be a file", ErrInvalidConfig, name) } return os.DirFS(dir), filepath.ToSlash(base), nil } diff --git a/engine_test.go b/engine_test.go index 9b005bd..2038929 100644 --- a/engine_test.go +++ b/engine_test.go @@ -2374,6 +2374,116 @@ func TestRunStructuredOutputWorksWithSchemaFile(t *testing.T) { } } +func TestSingleFileOptionsPreserveWhitespaceInPaths(t *testing.T) { + profile := promptkit.Profile{ + ID: "exact-path-profile", + Endpoint: "http://example.test/v1", + Model: "exact-path-model", + } + promptDocument := []byte(` +id: exact-path-prompt +version: "1.0.0" +default_profile: exact-path-profile +messages: + - role: user + content: exact path +output: + format: text + validation_mode: none +`) + profileDocument := []byte(` +id: exact-path-profile +endpoint: http://example.test/v1 +model: exact-path-model +`) + + tests := []struct { + name string + baseName string + content []byte + engine func(string) (*promptkit.Engine, error) + }{ + { + name: "prompt", + baseName: "prompt.yaml", + content: promptDocument, + engine: func(path string) (*promptkit.Engine, error) { + return promptkit.NewEngine(promptkit.Config{}, + promptkit.WithPromptFile(path), + promptkit.WithProfiles(profile), + ) + }, + }, + { + name: "profile", + baseName: "profile.yaml", + content: profileDocument, + engine: func(path string) (*promptkit.Engine, error) { + return promptkit.NewEngine(promptkit.Config{}, + promptkit.WithPromptFS(contractPromptFS("exact-path-prompt", "exact-path-profile", "exact path"), "."), + promptkit.WithProfileFile(path), + ) + }, + }, + { + name: "schema", + baseName: "schema.json", + content: []byte(publicSchemaJSON()), + engine: func(path string) (*promptkit.Engine, error) { + promptSource := fstest.MapFS{ + "prompt.yaml": &fstest.MapFile{Data: []byte(fmt.Sprintf(` +id: exact-path-prompt +version: "1.0.0" +default_profile: exact-path-profile +messages: + - role: user + content: exact path +output: + format: json + validation_mode: json_schema + schema_path: %q +`, filepath.Base(path)))}, + } + return promptkit.NewEngine(promptkit.Config{}, + promptkit.WithPromptFS(promptSource, "."), + promptkit.WithProfiles(profile), + promptkit.WithSchemaFile(path), + ) + }, + }, + } + positions := []struct { + name string + prefix string + suffix string + }{ + {name: "leading whitespace", prefix: " "}, + {name: "trailing whitespace", suffix: " "}, + } + + for _, tc := range tests { + for _, position := range positions { + t.Run(tc.name+"/"+position.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), position.prefix+tc.baseName+position.suffix) + if err := os.WriteFile(path, tc.content, 0o644); err != nil { + t.Fatalf("write source file: %v", err) + } + engine, err := tc.engine(path) + if err != nil { + t.Fatalf("construct engine from exact path %q: %v", path, err) + } + prepared, err := engine.Prepare(context.Background(), promptkit.RunRequest{PromptID: "exact-path-prompt"}) + if err != nil { + t.Fatalf("prepare from exact path %q: %v", path, err) + } + if prepared.EffectiveModelParams.Model != "exact-path-model" { + t.Fatalf("prepared model = %q, want exact-path-model", prepared.EffectiveModelParams.Model) + } + }) + } + } +} + func TestSourceOptionsRejectInvalidInputs(t *testing.T) { missingFile := filepath.Join(t.TempDir(), "missing.yaml") directoryPath := t.TempDir() @@ -2405,6 +2515,16 @@ func TestSourceOptionsRejectInvalidInputs(t *testing.T) { } }) } + + t.Run("later valid replacement does not hide invalid option", func(t *testing.T) { + _, err := promptkit.NewEngine(promptkit.Config{PromptDir: frameworkPromptDir}, + promptkit.WithProfileFS(nil, "profiles"), + promptkit.WithProfileFS(contractProfileFS("profile", "model"), "."), + ) + if !errors.Is(err, promptkit.ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig, got %v", err) + } + }) } func TestPackageOptionsComposeFromSlice(t *testing.T) { diff --git a/internal/defaults/defaults.go b/internal/defaults/defaults.go index 9274f65..07b770b 100644 --- a/internal/defaults/defaults.go +++ b/internal/defaults/defaults.go @@ -15,10 +15,7 @@ const ( OpenAIChatCompletionsPath = "/chat/completions" ExecutionDefaultTimeoutSeconds = 600 -) - -var ( - LLMRequestTimeoutDefault = 10 * time.Minute + LLMRequestTimeoutDefault = 10 * time.Minute ) func ExecutionTargetDefault() domain.ExecutionTarget { diff --git a/internal/filecatalog/catalog.go b/internal/filecatalog/catalog.go index e0c8369..904e393 100644 --- a/internal/filecatalog/catalog.go +++ b/internal/filecatalog/catalog.go @@ -36,7 +36,8 @@ func FindYAMLFiles(ctx context.Context, root string) ([]string, error) { return files, err } -// FindFSYAMLFiles returns sorted paths for .yaml and .yml files under root in fsys. +// FindFSYAMLFiles returns root itself when it names a file. For a directory +// root, it returns sorted paths for .yaml and .yml files beneath that root. func FindFSYAMLFiles(ctx context.Context, fsys fs.FS, root string) ([]string, error) { cleanRoot := CleanFSRoot(root) var files []string @@ -52,6 +53,10 @@ func FindFSYAMLFiles(ctx context.Context, fsys fs.FS, root string) ([]string, er if d.IsDir() { return nil } + if name == cleanRoot { + files = append(files, name) + return nil + } if !IsYAMLFile(d.Name()) { return nil } @@ -71,10 +76,10 @@ func RelativePath(root string, filePath string) string { return filepath.Clean(rel) } -// CleanFSRoot normalizes a root path for use with fs.FS. +// CleanFSRoot normalizes a root path for use with fs.FS while preserving +// nonblank leading and trailing whitespace. func CleanFSRoot(root string) string { - root = strings.TrimSpace(root) - if root == "" || root == "." { + if strings.TrimSpace(root) == "" || root == "." { return "." } return path.Clean(root) diff --git a/internal/filecatalog/catalog_test.go b/internal/filecatalog/catalog_test.go index 6a2a2cc..d3c72ce 100644 --- a/internal/filecatalog/catalog_test.go +++ b/internal/filecatalog/catalog_test.go @@ -54,7 +54,7 @@ func TestFindFSYAMLFilesNestedSortedAndFiltered(t *testing.T) { "other/ignored.yaml": &fstest.MapFile{Data: []byte("id: ignored")}, } - got, err := FindFSYAMLFiles(context.Background(), fsys, " prompts ") + got, err := FindFSYAMLFiles(context.Background(), fsys, "prompts") if err != nil { t.Fatalf("expected no error, got %v", err) } @@ -98,8 +98,11 @@ func TestCleanFSRoot(t *testing.T) { want string }{ {name: "empty", root: "", want: "."}, + {name: "whitespace only", root: " \t ", want: "."}, {name: "dot", root: ".", want: "."}, - {name: "trimmed", root: " prompts/../profiles ", want: "profiles"}, + {name: "cleaned", root: "prompts/../profiles", want: "profiles"}, + {name: "leading whitespace preserved", root: " profiles", want: " profiles"}, + {name: "trailing whitespace preserved", root: "profiles ", want: "profiles "}, } for _, tc := range tests { diff --git a/internal/validate/standard_validator.go b/internal/validate/standard_validator.go index f1df5c7..7877178 100644 --- a/internal/validate/standard_validator.go +++ b/internal/validate/standard_validator.go @@ -392,7 +392,7 @@ func (v *FSValidator) resolveSchemaPath(schemaPath string) (string, error) { if err != nil { return "", err } - if cleanSchemaPath != path.Base(cleanRoot) { + if cleanSchemaPath != strings.TrimSpace(path.Base(cleanRoot)) { return "", fmt.Errorf("schema path %q does not match schema file %q", cleanSchemaPath, path.Base(cleanRoot)) } resolved = cleanRoot diff --git a/public_contract_test.go b/public_contract_test.go index 34b2d78..f308db3 100644 --- a/public_contract_test.go +++ b/public_contract_test.go @@ -992,9 +992,9 @@ func TestFallbackProfileSourcePrecedence(t *testing.T) { t.Run("in-memory profiles override ordinary and fallback profiles", func(t *testing.T) { engine, err := promptkit.NewEngine(promptkit.Config{}, promptkit.WithPromptFS(contractPromptFS("prompt", profileID, "message"), "."), - promptkit.WithFallbackProfileFS(contractProfileFS(profileID, "fallback-model"), "."), - promptkit.WithProfileFS(contractProfileFS(profileID, "ordinary-model"), "."), promptkit.WithProfiles(promptkit.Profile{ID: profileID, Endpoint: "http://example.test/v1", Model: "memory-model"}), + promptkit.WithProfileFS(contractProfileFS(profileID, "ordinary-model"), "."), + promptkit.WithFallbackProfileFS(contractProfileFS(profileID, "fallback-model"), "."), ) if err != nil { t.Fatalf("construct engine: %v", err) @@ -1007,8 +1007,8 @@ func TestFallbackProfileSourcePrecedence(t *testing.T) { t.Run("ordinary filesystem source overrides fallback profile", func(t *testing.T) { engine, err := promptkit.NewEngine(promptkit.Config{}, promptkit.WithPromptFS(contractPromptFS("prompt", profileID, "message"), "."), - promptkit.WithFallbackProfileFS(contractProfileFS(profileID, "fallback-model"), "."), promptkit.WithProfileFS(contractProfileFS(profileID, "ordinary-model"), "."), + promptkit.WithFallbackProfileFS(contractProfileFS(profileID, "fallback-model"), "."), ) if err != nil { t.Fatalf("construct engine: %v", err) @@ -1018,6 +1018,21 @@ func TestFallbackProfileSourcePrecedence(t *testing.T) { } }) + t.Run("ordinary option replaces configured directory", func(t *testing.T) { + profileDir := t.TempDir() + writePublicProfileFile(t, profileDir, profileID, "http://example.test/v1", "directory-model") + engine, err := promptkit.NewEngine(promptkit.Config{ProfileDir: profileDir}, + promptkit.WithPromptFS(contractPromptFS("prompt", profileID, "message"), "."), + promptkit.WithProfileFS(contractProfileFS(profileID, "option-model"), "."), + ) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + if model := prepareModel(t, engine, "prompt"); model != "option-model" { + t.Fatalf("expected ordinary option profile, got %q", model) + } + }) + t.Run("configured directory overrides fallback profile", func(t *testing.T) { profileDir := t.TempDir() writePublicProfileFile(t, profileDir, profileID, "http://example.test/v1", "directory-model")