Unify prompt repository source handling

docs/internal/sources.md

@@ -12,18 +12,21 @@ validation modes, built-in catalog, and source precedence.
 
 ## Prompt Definitions
 
-`internal/promptdef` discovers YAML deterministically, requires one strictly
-decoded document per file, and scans normalized ID and version metadata before
-validating the selected definition. It retains the point-in-time metadata scan
-needed for duplicate detection while opening file-backed message content only
-for one exact selected candidate. Content resolution uses an explicit
-source-root abstraction. Operating-system
-sources enforce containment against canonical roots and targets so symlinks
-cannot escape. Injected `fs.FS` sources enforce containment in their clean
-relative path namespace. A single-file source uses the selected prompt file's
-containing directory as its root. Every content path must be relative and is
-opened from its exact parsed text after a separate blank check; contained
-parent components and whitespace-bearing names remain valid.
+`internal/promptdef` uses one source-neutral flow for prompt selection and
+normalization. That flow scans normalized YAML ID and version metadata,
+requires one strictly decoded document per file, classifies errors for the
+selected definition, detects duplicates, and normalizes the exact match.
+Small operating-system and `fs.FS` adapters own discovery, byte reads, display
+paths, content opening, and root containment. Each lookup remains a
+point-in-time scan: definitions and catalogs are not cached, and file-backed
+message content is opened only for the exact selected candidate.
+
+Operating-system sources enforce containment against canonical roots and
+targets so symlinks cannot escape. Injected `fs.FS` sources enforce containment
+in their clean relative path namespace. A single-file source uses the selected
+prompt file's containing directory as its root. Every content path must be
+relative and is opened from its exact parsed text after a separate blank check;
+contained parent components and whitespace-bearing names remain valid.
 
 Exact prompt inspection performs one point-in-time lookup through that same
 repository and validates referenced message content before returning declared

internal/promptdef/filesystem_repository.go

@@ -7,11 +7,9 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
-	"os"
 	"strings"
 
 	"gitea.maximumdirect.net/eric/promptkit/internal/domain"
-	"gitea.maximumdirect.net/eric/promptkit/internal/filecatalog"
 	"gopkg.in/yaml.v3"
 )
 
@@ -21,15 +19,8 @@ var (
 	ErrInvalidPromptDefinition  = errors.New("invalid prompt definition configuration")
 )
 
-type filesystemRepository struct {
-	dir        string
-	sourceRoot contentSourceRoot
-}
-
-type fsRepository struct {
-	fsys       fs.FS
-	root       string
-	sourceRoot contentSourceRoot
+type sourceRepository struct {
+	source promptDefinitionSource
 }
 
 type promptDefinitionFile struct {
@@ -70,38 +61,47 @@ type promptOutputContractFile struct {
 }
 
 func NewFilesystemRepository(dir string) Repository {
-	return &filesystemRepository{
-		dir:        dir,
-		sourceRoot: osContentSourceRoot{root: dir},
+	return &sourceRepository{
+		source: osPromptSource{
+			root:        dir,
+			contentRoot: osContentSourceRoot{root: dir},
+		},
 	}
 }
 
 func NewFSRepository(fsys fs.FS, root string) Repository {
-	return &fsRepository{
-		fsys:       fsys,
-		root:       root,
-		sourceRoot: fsContentSourceRoot{fsys: fsys, root: root},
+	return &sourceRepository{
+		source: fsPromptSource{
+			fsys:        fsys,
+			root:        root,
+			contentRoot: fsContentSourceRoot{fsys: fsys, root: root},
+		},
 	}
 }
 
 // NewFileRepository constructs a repository for one operating-system prompt file.
 func NewFileRepository(fsys fs.FS, file string, sourceDir string) Repository {
-	return &fsRepository{
-		fsys: fsys,
-		root: file,
-		sourceRoot: osContentSourceRoot{
-			root:                sourceDir,
-			sourcePathsRelative: true,
+	return &sourceRepository{
+		source: fsPromptSource{
+			fsys: fsys,
+			root: file,
+			contentRoot: osContentSourceRoot{
+				root:                sourceDir,
+				sourcePathsRelative: true,
+			},
 		},
 	}
 }
 
-func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id string, version string) (*domain.PromptDefinition, error) {
+func (r *sourceRepository) GetPromptDefinition(ctx context.Context, id string, version string) (*domain.PromptDefinition, error) {
 	if strings.TrimSpace(id) == "" {
 		return nil, fmt.Errorf("%w: prompt id is required", ErrInvalidPromptDefinition)
 	}
+	if r == nil || r.source == nil {
+		return nil, errors.New("failed to read prompt definition directory: source is nil")
+	}
 
-	files, err := filecatalog.FindYAMLFiles(ctx, r.dir)
+	files, err := r.source.findYAMLFiles(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read prompt definition directory: %w", err)
 	}
@@ -114,8 +114,8 @@ func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id strin
 		default:
 		}
 
-		relPath := filecatalog.RelativePath(r.dir, fullPath)
-		data, err := os.ReadFile(fullPath)
+		relPath := r.source.displayPath(fullPath)
+		data, err := r.source.readDefinition(fullPath)
 		if err != nil {
 			continue
 		}
@@ -149,7 +149,7 @@ func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id strin
 
 	if len(matches) == 1 {
 		match := matches[0]
-		def, err := normalizePromptDefinition(match.raw, r.sourceRoot, match.sourcePath)
+		def, err := normalizePromptDefinition(match.raw, r.source, match.sourcePath)
 		if err != nil {
 			return nil, fmt.Errorf("%w: %s: %v", ErrInvalidPromptDefinition, match.path, err)
 		}
@@ -159,82 +159,12 @@ func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id strin
 	return nil, ErrPromptDefinitionNotFound
 }
 
-func (r *fsRepository) GetPromptDefinition(ctx context.Context, id string, version string) (*domain.PromptDefinition, error) {
-	return loadPromptDefinition(ctx, r.fsys, r.root, r.sourceRoot, id, version)
-}
-
 type promptDefinitionMatch struct {
 	raw        *promptDefinitionFile
 	sourcePath string
 	path       string
 }
 
-func loadPromptDefinition(ctx context.Context, fsys fs.FS, root string, sourceRoot contentSourceRoot, id string, version string) (*domain.PromptDefinition, error) {
-	if strings.TrimSpace(id) == "" {
-		return nil, fmt.Errorf("%w: prompt id is required", ErrInvalidPromptDefinition)
-	}
-	if fsys == nil {
-		return nil, fmt.Errorf("failed to read prompt definition directory: filesystem is nil")
-	}
-
-	files, err := filecatalog.FindFSYAMLFiles(ctx, fsys, root)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read prompt definition directory: %w", err)
-	}
-	var matches []promptDefinitionMatch
-	for _, fullPath := range files {
-		select {
-		case <-ctx.Done():
-			return nil, ctx.Err()
-		default:
-		}
-
-		relPath := filecatalog.DisplayPath(root, fullPath)
-		data, err := fs.ReadFile(fsys, fullPath)
-		if err != nil {
-			continue
-		}
-
-		raw, err := decodePromptDefinition(data)
-		if err != nil {
-			if promptDefinitionDataMatches(data, id, version) {
-				return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, err)
-			}
-			continue
-		}
-		if !promptDefinitionMatches(raw, id, version) {
-			continue
-		}
-		matches = append(matches, promptDefinitionMatch{
-			raw:        raw,
-			sourcePath: fullPath,
-			path:       relPath,
-		})
-	}
-
-	if len(matches) > 1 {
-		paths := make([]string, 0, len(matches))
-		for _, match := range matches {
-			paths = append(paths, match.path)
-		}
-		if version != "" {
-			return nil, fmt.Errorf("%w: duplicate prompt definition id %q version %q found in: %s", ErrInvalidPromptDefinition, id, version, strings.Join(paths, ", "))
-		}
-		return nil, fmt.Errorf("%w: duplicate prompt definition id %q found in: %s", ErrInvalidPromptDefinition, id, strings.Join(paths, ", "))
-	}
-
-	if len(matches) == 1 {
-		match := matches[0]
-		def, err := normalizePromptDefinition(match.raw, sourceRoot, match.sourcePath)
-		if err != nil {
-			return nil, fmt.Errorf("%w: %s: %v", ErrInvalidPromptDefinition, match.path, err)
-		}
-		return def, nil
-	}
-
-	return nil, ErrPromptDefinitionNotFound
-}
-
 func decodePromptDefinition(data []byte) (*promptDefinitionFile, error) {
 	var raw promptDefinitionFile
 	decoder := yaml.NewDecoder(bytes.NewReader(data))

internal/promptdef/repository_test.go

@@ -3,6 +3,7 @@ package promptdef
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -14,13 +15,39 @@ import (
 	"gitea.maximumdirect.net/eric/promptkit/internal/domain"
 )
 
-func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
+func TestPromptRepositoryDefinitionFixtures(t *testing.T) {
+	sources := []struct {
+		name                string
+		newRepository       func(string) Repository
+		contentPathsAreFull bool
+	}{
+		{
+			name:                "operating system",
+			newRepository:       NewFilesystemRepository,
+			contentPathsAreFull: true,
+		},
+		{
+			name: "filesystem",
+			newRepository: func(root string) Repository {
+				return NewFSRepository(os.DirFS(root), ".")
+			},
+		},
+	}
+
+	for _, source := range sources {
+		t.Run(source.name, func(t *testing.T) {
+			testPromptRepositoryDefinitionFixtures(t, source.newRepository, source.contentPathsAreFull)
+		})
+	}
+}
+
+func testPromptRepositoryDefinitionFixtures(t *testing.T, newRepository func(string) Repository, contentPathsAreFull bool) {
 	tmpDir := t.TempDir()
 	if err := copyTree("testdata", tmpDir); err != nil {
 		t.Fatalf("failed to copy testdata: %v", err)
 	}
 
-	repo := NewFilesystemRepository(tmpDir)
+	repo := newRepository(tmpDir)
 	ctx := context.Background()
 
 	t.Run("valid inline prompt", func(t *testing.T) {
@@ -65,8 +92,8 @@ func TestFilesystemRepository_GetPromptDefinition(t *testing.T) {
 		if p.Templates[1].ContentFile == "" {
 			t.Fatal("expected ContentFile source metadata to be preserved")
 		}
-		if !filepath.IsAbs(p.Templates[1].ContentFile) {
-			t.Fatalf("expected resolved content_file path to be absolute, got %q", p.Templates[1].ContentFile)
+		if filepath.IsAbs(p.Templates[1].ContentFile) != contentPathsAreFull {
+			t.Fatalf("unexpected content_file path representation: %q", p.Templates[1].ContentFile)
 		}
 	})
 
@@ -456,63 +483,6 @@ func (f *recordingFS) openCount(name string) int {
 	return count
 }
 
-func TestFSRepositoryRejectsDuplicatePromptIDs(t *testing.T) {
-	repo := NewFSRepository(fstest.MapFS{
-		"one.yaml": &fstest.MapFile{Data: []byte(`
-id: duplicate-fs-prompt
-version: "1.0.0"
-messages:
-  - role: user
-    content: First.
-output:
-  format: text
-  validation_mode: none
-  repair_attempts: 0
-`)},
-		"nested/two.yaml": &fstest.MapFile{Data: []byte(`
-id: duplicate-fs-prompt
-version: "1.0.0"
-messages:
-  - role: user
-    content: Second.
-output:
-  format: text
-  validation_mode: none
-  repair_attempts: 0
-`)},
-	}, ".")
-
-	_, err := repo.GetPromptDefinition(context.Background(), "duplicate-fs-prompt", "")
-	if !errors.Is(err, ErrInvalidPromptDefinition) {
-		t.Fatalf("expected ErrInvalidPromptDefinition, got %v", err)
-	}
-	if !strings.Contains(err.Error(), "one.yaml") || !strings.Contains(err.Error(), "nested/two.yaml") {
-		t.Fatalf("expected duplicate paths in error, got %v", err)
-	}
-}
-
-func TestFSRepositoryRejectsUnknownYAMLFields(t *testing.T) {
-	repo := NewFSRepository(fstest.MapFS{
-		"not_named_like_id.yaml": &fstest.MapFile{Data: []byte(`
-id: strict-fs-prompt
-version: "1.0.0"
-unknown: true
-messages:
-  - role: user
-    content: Invalid.
-output:
-  format: text
-  validation_mode: none
-  repair_attempts: 0
-`)},
-	}, ".")
-
-	_, err := repo.GetPromptDefinition(context.Background(), "strict-fs-prompt", "")
-	if !errors.Is(err, ErrInvalidYAML) {
-		t.Fatalf("expected ErrInvalidYAML, got %v", err)
-	}
-}
-
 func TestPromptRepositorySelectionUsesYAMLMetadata(t *testing.T) {
 	const validDefinition = `
 id: selected-prompt
@@ -529,6 +499,7 @@ output:
 		files       map[string]string
 		wantErr     error
 		diagnostics []string
+		wantContent string
 	}{
 		{
 			name: "same-stem strict error with different YAML ID is unrelated",
@@ -561,6 +532,41 @@ output:
 				"valid.yaml": validDefinition,
 			},
 		},
+		{
+			name: "selected content is resolved relative to its definition",
+			files: map[string]string{
+				"nested/selected.yaml": `
+id: selected-prompt
+version: "1"
+messages:
+  - role: user
+    content_file: ./content/selected.tmpl
+output:
+  format: text
+  validation_mode: none
+`,
+				"nested/content/selected.tmpl": "selected from file",
+			},
+			wantContent: "selected from file",
+		},
+		{
+			name: "duplicate selected definitions are ambiguous",
+			files: map[string]string{
+				"selected-a.yaml": validDefinition,
+				"nested/selected-b.yaml": `
+id: selected-prompt
+version: "1"
+messages:
+  - role: user
+    content: duplicate
+output:
+  format: text
+  validation_mode: none
+`,
+			},
+			wantErr:     ErrInvalidPromptDefinition,
+			diagnostics: []string{"duplicate prompt definition id", "selected-a.yaml", "nested/selected-b.yaml"},
+		},
 		{
 			name: "selected strict error is authoritative",
 			files: map[string]string{
@@ -587,6 +593,23 @@ output:
 			wantErr:     ErrInvalidPromptDefinition,
 			diagnostics: []string{"selected-invalid.yaml", "at least one message"},
 		},
+		{
+			name: "selected content error includes definition context",
+			files: map[string]string{
+				"selected-missing-content.yaml": `
+id: selected-prompt
+version: "1"
+messages:
+  - role: user
+    content_file: missing.tmpl
+output:
+  format: text
+  validation_mode: none
+`,
+			},
+			wantErr:     ErrInvalidPromptDefinition,
+			diagnostics: []string{"selected-missing-content.yaml", "failed to read content_file", "missing.tmpl"},
+		},
 	}
 
 	for _, source := range promptRepositorySources() {
@@ -608,7 +631,11 @@ output:
 				if err != nil {
 					t.Fatalf("load selected prompt: %v", err)
 				}
-				if got.ID != "selected-prompt" || got.Version != "1" || got.Templates[0].Content != "selected" {
+				wantContent := tc.wantContent
+				if wantContent == "" {
+					wantContent = "selected"
+				}
+				if got.ID != "selected-prompt" || got.Version != "1" || got.Templates[0].Content != wantContent {
 					t.Fatalf("unexpected selected definition: %+v", got)
 				}
 			})
@@ -616,6 +643,32 @@ output:
 	}
 }
 
+func TestPromptRepositoryHonorsCancellation(t *testing.T) {
+	for _, source := range promptRepositorySources() {
+		t.Run(source.name, func(t *testing.T) {
+			repo := source.newRepository(t, map[string]string{
+				"definition.yaml": `
+id: cancelled-prompt
+version: "1"
+messages:
+  - role: user
+    content: selected
+output:
+  format: text
+  validation_mode: none
+`,
+			})
+			ctx, cancel := context.WithCancel(context.Background())
+			cancel()
+
+			_, err := repo.GetPromptDefinition(ctx, "cancelled-prompt", "1")
+			if !errors.Is(err, context.Canceled) {
+				t.Fatalf("expected context cancellation, got %v", err)
+			}
+		})
+	}
+}
+
 func TestPromptRepositoryRequiresOneYAMLDocument(t *testing.T) {
 	const definition = `
 id: one-document
@@ -902,6 +955,70 @@ func promptRepositorySources() []promptRepositorySource {
 	}
 }
 
+func BenchmarkPromptRepositoryLookup(b *testing.B) {
+	for _, size := range []int{10, 1000} {
+		b.Run(fmt.Sprintf("catalog-%d", size), func(b *testing.B) {
+			files := fstest.MapFS{
+				"target.yaml": &fstest.MapFile{Data: []byte(`
+id: target
+version: "1"
+messages:
+  - role: user
+    content_file: target.tmpl
+output:
+  format: text
+  validation_mode: none
+`)},
+				"target.tmpl": &fstest.MapFile{Data: []byte("selected")},
+			}
+			metadataNames := []string{"target.yaml"}
+			contentNames := make([]string, 0, size-1)
+			for i := 1; i < size; i++ {
+				definitionName := fmt.Sprintf("prompt-%04d.yaml", i)
+				contentName := fmt.Sprintf("prompt-%04d.tmpl", i)
+				files[definitionName] = &fstest.MapFile{Data: []byte(fmt.Sprintf(`
+id: prompt-%04d
+version: "1"
+messages:
+  - role: user
+    content_file: %s
+output:
+  format: text
+  validation_mode: none
+`, i, contentName))}
+				files[contentName] = &fstest.MapFile{Data: []byte("unrelated")}
+				metadataNames = append(metadataNames, definitionName)
+				contentNames = append(contentNames, contentName)
+			}
+
+			fsys := &recordingFS{FS: files}
+			repo := NewFSRepository(fsys, ".")
+			b.ReportAllocs()
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				if _, err := repo.GetPromptDefinition(context.Background(), "target", "1"); err != nil {
+					b.Fatal(err)
+				}
+			}
+			b.StopTimer()
+
+			if got := fsys.openCount("target.tmpl"); got != b.N {
+				b.Fatalf("selected content opens = %d, want %d", got, b.N)
+			}
+			for _, name := range contentNames {
+				if got := fsys.openCount(name); got != 0 {
+					b.Fatalf("unrelated content %q opened %d times", name, got)
+				}
+			}
+			for _, name := range metadataNames {
+				if got := fsys.openCount(name); got != b.N {
+					b.Fatalf("metadata %q opens = %d, want %d", name, got, b.N)
+				}
+			}
+		})
+	}
+}
+
 func assertCacheControl(t *testing.T, got *domain.CacheControl, wantType domain.CacheControlType, wantTTL string) {
 	t.Helper()
 	if got == nil {

internal/promptdef/source.go

@@ -0,0 +1,63 @@
+package promptdef
+
+import (
+	"context"
+	"errors"
+	"io/fs"
+	"os"
+
+	"gitea.maximumdirect.net/eric/promptkit/internal/filecatalog"
+)
+
+type promptDefinitionSource interface {
+	contentSourceRoot
+	findYAMLFiles(context.Context) ([]string, error)
+	readDefinition(string) ([]byte, error)
+	displayPath(string) string
+}
+
+type osPromptSource struct {
+	root        string
+	contentRoot osContentSourceRoot
+}
+
+func (s osPromptSource) findYAMLFiles(ctx context.Context) ([]string, error) {
+	return filecatalog.FindYAMLFiles(ctx, s.root)
+}
+
+func (s osPromptSource) readDefinition(name string) ([]byte, error) {
+	return os.ReadFile(name)
+}
+
+func (s osPromptSource) displayPath(name string) string {
+	return filecatalog.RelativePath(s.root, name)
+}
+
+func (s osPromptSource) readContentFile(sourcePath string, contentFile string) (string, string, error) {
+	return s.contentRoot.readContentFile(sourcePath, contentFile)
+}
+
+type fsPromptSource struct {
+	fsys        fs.FS
+	root        string
+	contentRoot contentSourceRoot
+}
+
+func (s fsPromptSource) findYAMLFiles(ctx context.Context) ([]string, error) {
+	if s.fsys == nil {
+		return nil, errors.New("filesystem is nil")
+	}
+	return filecatalog.FindFSYAMLFiles(ctx, s.fsys, s.root)
+}
+
+func (s fsPromptSource) readDefinition(name string) ([]byte, error) {
+	return fs.ReadFile(s.fsys, name)
+}
+
+func (s fsPromptSource) displayPath(name string) string {
+	return filecatalog.DisplayPath(s.root, name)
+}
+
+func (s fsPromptSource) readContentFile(sourcePath string, contentFile string) (string, string, error) {
+	return s.contentRoot.readContentFile(sourcePath, contentFile)
+}