Share bounded regular file read mechanics

docs/internal/overview.md

@@ -6,7 +6,8 @@ module and package documentation boundary. Its implemented public primitives are
 - immutable semantic `Path` construction and strict JSON Pointer parsing;
 - detached provenance and diagnostic record types with empty `Metadata` queries;
 - canonical, control-safe diagnostic rendering and structured `LoadError` access;
-- opaque file, byte, reader, and no-input source construction and bounded reads;
+- opaque file, byte, reader, and no-input source construction in `source.go`,
+  with shared regular-file checks and bounded read mechanics in `source_file.go`;
 - a parser-independent ordered tree in `internal/document`;
 - the sole upstream YAML integration in `internal/yaml`, which translates validated
   input into that owned tree and exposes no parser types outside its boundary;

docs/roadmap/remediation.md

@@ -120,7 +120,14 @@ small cross-entry regression only if a source path lacks equivalent coverage.
 
 ## Stage 3 — Share bounded regular-file read mechanics
 
-Status: pending.
+Status: complete.
+
+Completion: `source_file.go` owns regular-file preflight/open/recheck/close and
+bounded reads, with private failure kinds and stat/open-only optional-missing
+classification. Callers retain containment, metadata, budgets, accounting, and
+limit diagnostics. Existing filesystem, FIFO, budget, and reader tests pass;
+a private regression covers nondeterministic filesystem failure phases. Full
+offline baseline and race suites passed with Go 1.26.7 on Linux/amd64.
 
 Evidence: `readSource`, `importTraversal.readImport`, and `readProfileFile` repeat
 preflight stat, open, post-open stat, regular-file checks, size checks, and bounded

imports.go

@@ -3,7 +3,6 @@ package configkit
 import (
 	"bytes"
 	"errors"
-	"io"
 	"io/fs"
 	"os"
 	"path"
@@ -382,47 +381,12 @@ func (s *importTraversal) readImport(relative, sourceID string, optional bool) (
 	if limit < 0 {
 		limit = 0
 	}
-	preflight, err := s.root.Stat(relative)
-	if err != nil {
-		if optional && errors.Is(err, fs.ErrNotExist) {
+	data, failure := readRegularFile(relative, s.root.Stat, s.root.Open, limit)
+	if failure != nil {
+		if optional && failure.missingTarget {
 			return nil, true, nil
 		}
-		value := classifySourceError(err, sourceID)
-		return nil, false, &value
-	}
-	if !preflight.Mode().IsRegular() {
-		value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")
-		return nil, false, &value
-	}
-	file, err := s.root.Open(relative)
-	if err != nil {
-		if optional && errors.Is(err, fs.ErrNotExist) {
-			return nil, true, nil
-		}
-		value := classifySourceError(err, sourceID)
-		return nil, false, &value
-	}
-	defer func() { _ = file.Close() }()
-	info, err := file.Stat()
-	if err != nil {
-		value := classifySourceError(err, sourceID)
-		return nil, false, &value
-	}
-	if !info.Mode().IsRegular() {
-		value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")
-		return nil, false, &value
-	}
-	if info.Size() > limit {
-		value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit", &Location{SourceID: sourceID})
-		return nil, false, &value
-	}
-	data, err := io.ReadAll(io.LimitReader(file, limit+1))
-	if err != nil {
-		value := classifySourceError(err, sourceID)
-		return nil, false, &value
-	}
-	if int64(len(data)) > limit {
-		value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit", &Location{SourceID: sourceID})
+		value := sourceReadDiagnostic(failure, sourceID, CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit")
 		return nil, false, &value
 	}
 	s.usedBytes += int64(len(data))

profile.go

@@ -3,7 +3,6 @@ package configkit
 import (
 	"bytes"
 	"errors"
-	"io"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -122,41 +121,9 @@ func readProfileFile(root *os.Root, relative, sourceID string, remaining int64)
 	if limit < 0 {
 		limit = 0
 	}
-	preflight, err := root.Stat(relative)
-	if err != nil {
-		value := classifySourceError(err, sourceID)
-		return nil, &value
-	}
-	if !preflight.Mode().IsRegular() {
-		value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")
-		return nil, &value
-	}
-	file, err := root.Open(relative)
-	if err != nil {
-		value := classifySourceError(err, sourceID)
-		return nil, &value
-	}
-	defer func() { _ = file.Close() }()
-	info, err := file.Stat()
-	if err != nil {
-		value := classifySourceError(err, sourceID)
-		return nil, &value
-	}
-	if !info.Mode().IsRegular() {
-		value := sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")
-		return nil, &value
-	}
-	if info.Size() > limit {
-		value := importDiagnostic(CodeProfileLimitExceeded, "selected profile exceeds a source or aggregate byte limit", &Location{SourceID: sourceID})
-		return nil, &value
-	}
-	data, err := io.ReadAll(io.LimitReader(file, limit+1))
-	if err != nil {
-		value := classifySourceError(err, sourceID)
-		return nil, &value
-	}
-	if int64(len(data)) > limit {
-		value := importDiagnostic(CodeProfileLimitExceeded, "selected profile exceeds a source or aggregate byte limit", &Location{SourceID: sourceID})
+	data, failure := readRegularFile(relative, root.Stat, root.Open, limit)
+	if failure != nil {
+		value := sourceReadDiagnostic(failure, sourceID, CodeProfileLimitExceeded, "selected profile exceeds a source or aggregate byte limit")
 		return nil, &value
 	}
 	return data, nil

source.go

@@ -125,10 +125,7 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead
 	}
 	info := &SourceInfo{ID: sourceID, Label: source.label, Layer: LayerBase}
 
-	var (
-		input io.Reader
-		close func() error
-	)
+	var input io.Reader
 	switch source.kind {
 	case sourceFile:
 		absolute, err := filepath.Abs(source.path)
@@ -141,32 +138,11 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead
 			return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)}
 		}
 		info.Path = canonical
-		preflight, err := os.Stat(canonical)
-		if err != nil {
-			return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)}
+		data, failure := readRegularFile(canonical, os.Stat, os.Open, limit)
+		if failure != nil {
+			return sourceRead{info: info}, []Diagnostic{sourceReadDiagnostic(failure, sourceID, CodeSourceLimitExceeded, "source exceeds the configured byte limit")}
 		}
-		if !preflight.Mode().IsRegular() {
-			return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")}
-		}
-		file, err := os.Open(canonical)
-		if err != nil {
-			return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)}
-		}
-		close = file.Close
-		stat, err := file.Stat()
-		if err != nil {
-			_ = file.Close()
-			return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)}
-		}
-		if !stat.Mode().IsRegular() {
-			_ = file.Close()
-			return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")}
-		}
-		if stat.Size() > limit {
-			_ = file.Close()
-			return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceLimitExceeded, sourceID, "source exceeds the configured byte limit")}
-		}
-		input = file
+		return sourceRead{data: bytes.Clone(data), info: info}, nil
 	case sourceBytes:
 		input = bytes.NewReader(source.data)
 	case sourceReader:
@@ -177,16 +153,9 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead
 	default:
 		return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceInvalid, sourceID, "source is invalid")}
 	}
-	if close != nil {
-		defer func() { _ = close() }()
-	}
-
-	data, err := io.ReadAll(io.LimitReader(input, limit+1))
-	if err != nil {
-		return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)}
-	}
-	if int64(len(data)) > limit {
-		return sourceRead{info: info}, []Diagnostic{sourceDiagnostic(CodeSourceLimitExceeded, sourceID, "source exceeds the configured byte limit")}
+	data, failure := readBoundedSource(input, limit)
+	if failure != nil {
+		return sourceRead{info: info}, []Diagnostic{sourceReadDiagnostic(failure, sourceID, CodeSourceLimitExceeded, "source exceeds the configured byte limit")}
 	}
 	return sourceRead{data: bytes.Clone(data), info: info}, nil
 }

source_file.go

@@ -0,0 +1,75 @@
+package configkit
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+	"os"
+)
+
+type sourceReadFailureKind uint8
+
+const (
+	sourceReadIO sourceReadFailureKind = iota
+	sourceReadNotRegular
+	sourceReadLimit
+)
+
+// sourceReadFailure never escapes the private source boundary. missingTarget is
+// set only for preflight/open failures; a later missing-file error is not optional.
+type sourceReadFailure struct {
+	kind          sourceReadFailureKind
+	cause         error
+	missingTarget bool
+}
+
+// readRegularFile borrows stat/open operations so root-bound callers retain their
+// containment policy. Preflight rejects FIFOs before open can block. The opened
+// handle is checked again and always closed; read-only close errors are ignored.
+func readRegularFile(name string, stat func(string) (fs.FileInfo, error), open func(string) (*os.File, error), limit int64) ([]byte, *sourceReadFailure) {
+	info, err := stat(name)
+	if err != nil {
+		return nil, &sourceReadFailure{cause: err, missingTarget: errors.Is(err, fs.ErrNotExist)}
+	}
+	if !info.Mode().IsRegular() {
+		return nil, &sourceReadFailure{kind: sourceReadNotRegular}
+	}
+	file, err := open(name)
+	if err != nil {
+		return nil, &sourceReadFailure{cause: err, missingTarget: errors.Is(err, fs.ErrNotExist)}
+	}
+	defer func() { _ = file.Close() }()
+	info, err = file.Stat()
+	if err != nil {
+		return nil, &sourceReadFailure{cause: err}
+	}
+	if !info.Mode().IsRegular() {
+		return nil, &sourceReadFailure{kind: sourceReadNotRegular}
+	}
+	if info.Size() > limit {
+		return nil, &sourceReadFailure{kind: sourceReadLimit}
+	}
+	return readBoundedSource(file, limit)
+}
+
+func readBoundedSource(input io.Reader, limit int64) ([]byte, *sourceReadFailure) {
+	data, err := io.ReadAll(io.LimitReader(input, limit+1))
+	if err != nil {
+		return nil, &sourceReadFailure{cause: err}
+	}
+	if int64(len(data)) > limit {
+		return nil, &sourceReadFailure{kind: sourceReadLimit}
+	}
+	return data, nil
+}
+
+func sourceReadDiagnostic(failure *sourceReadFailure, sourceID string, limitCode Code, limitMessage string) Diagnostic {
+	switch failure.kind {
+	case sourceReadNotRegular:
+		return sourceDiagnostic(CodeSourceNotRegular, sourceID, "source is not a regular file")
+	case sourceReadLimit:
+		return sourceDiagnostic(limitCode, sourceID, limitMessage)
+	default:
+		return classifySourceError(failure.cause, sourceID)
+	}
+}

source_file_test.go

@@ -0,0 +1,48 @@
+package configkit
+
+import (
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// Files disappearing between filesystem operations are not deterministic with
+// real concurrent deletion. Exercise the private seam that controls optionality.
+func TestRegularFileMissingFailurePhases(t *testing.T) {
+	name := filepath.Join(t.TempDir(), "input.yml")
+	if err := os.WriteFile(name, []byte("{}"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	for _, test := range []struct {
+		name    string
+		stat    func(string) (fs.FileInfo, error)
+		open    func(string) (*os.File, error)
+		missing bool
+	}{
+		{"preflight", func(string) (fs.FileInfo, error) { return nil, fs.ErrNotExist }, os.Open, true},
+		{"open", os.Stat, func(string) (*os.File, error) { return nil, fs.ErrNotExist }, true},
+		{"post-open stat", os.Stat, func(name string) (*os.File, error) {
+			file, err := os.Open(name)
+			if err == nil {
+				err = file.Close()
+			}
+			return file, err
+		}, false},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			data, failure := readRegularFile(name, test.stat, test.open, 16)
+			if data != nil || failure == nil || failure.kind != sourceReadIO || failure.missingTarget != test.missing {
+				t.Fatalf("data = %v, failure = %#v", data, failure)
+			}
+		})
+	}
+	_, failure := readBoundedSource(missingDuringRead{}, 16)
+	if failure == nil || failure.kind != sourceReadIO || failure.missingTarget {
+		t.Fatalf("late read failure = %#v", failure)
+	}
+}
+
+type missingDuringRead struct{}
+
+func (missingDuringRead) Read([]byte) (int, error) { return 0, fs.ErrNotExist }