eric/weatherreporter
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Reduce comparison bundle recognition reads

Browse Source
This commit is contained in:
Eric Rakestraw
2026-08-13 03:52:20 +00:00
parent 0516ee148d
commit fb891fad07
4 changed files with 210 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
docs/internal/comparison-publication.md
View File

@@ -15,12 +15,13 @@ filesystem validation then require that exact path and file set.
Destination planning is read-only. It requires an exact absolute target that
is neither the filesystem root nor the working directory, rejects unsafe
symlinks and non-directories, accepts a missing or empty directory, and permits
replacement only for a recognized current bundle. Publication rechecks that
authorization immediately before it writes a private sibling staging directory.
For replacement, it moves the prior bundle to a private sibling backup,
reauthorizes that moved entry, checks for cancellation, and restores it if
cancellation or installing the new bundle prevents replacement. If guarded
restoration fails, the error retains the prior bundle's recovery path.
replacement only for a recognized current bundle. Publication rechecks the
destination namespace and type immediately before it writes a private sibling
staging directory. For replacement, it moves the prior bundle to a private
sibling backup, fully reauthorizes that moved entry, checks for cancellation,
and restores it if cancellation or installing the new bundle prevents
replacement. If guarded restoration fails, the error retains the prior bundle's
recovery path.
Planning also validates the final component and the bounded fixed names used
for private staging and backup siblings. A destination that cannot form those
@@ -37,8 +38,9 @@ complete recognized recovery bundle, partial remnants, an absent sibling, or
an uninspectable state. A recovery path is reported only when something
remains; only a complete recognized bundle is suitable for rollback recovery.
The application preflights before prompt inspection and collection, then
preflights again before publication. A cancellation or any failure before the
The application preflights before prompt inspection and collection. Publication
performs its transaction-boundary checks and final moved-destination
authorization before installation. A cancellation or any failure before the
commit leaves the prior destination untouched. Completed bundles include
partial profile results; comparison publication never coordinates Distributor
notification. Operator-facing lifecycle and cleanup are in the

6
internal/app/comparison.go
View File

@@ -110,7 +110,7 @@ func compareDetailed(ctx context.Context, req ComparisonRequest, publish compari
return result, err
}
result.OutputDirectory = outputDirectory
_, err = comparison.PlanDestination(req.WorkingDir, outputDirectory, req.Replace)
publicationPlan, err := comparison.PlanDestination(req.WorkingDir, outputDirectory, req.Replace)
if err != nil {
return result, fmt.Errorf("preflight comparison destination: %w", err)
}
@@ -150,10 +150,6 @@ func compareDetailed(ctx context.Context, req ComparisonRequest, publish compari
if err := bundle.Validate(); err != nil {
return result, fmt.Errorf("build comparison bundle: %w", err)
}
publicationPlan, err := comparison.PlanDestination(req.WorkingDir, outputDirectory, req.Replace)
if err != nil {
return result, fmt.Errorf("re-preflight comparison destination: %w", err)
}
publication, err := publish(ctx, publicationPlan, bundle)
if publication.Committed {
result.OutputDirectory = publicationPlan.Target

111
internal/comparison/publish.go
View File

@@ -73,6 +73,18 @@ var ErrUnrecognizedBundle = errors.New("unrecognized comparison bundle")
// PlanDestination performs the read-only comparison destination preflight.
func PlanDestination(workingDirectory, target string, replace bool) (DestinationPlan, error) {
return planDestination(workingDirectory, target, replace, RecognizeBundle)
}
func planDestination(workingDirectory, target string, replace bool, recognize func(string) (Manifest, error)) (DestinationPlan, error) {
plan, err := newDestinationPlan(workingDirectory, target, replace)
if err != nil {
return DestinationPlan{}, err
}
return inspectDestination(plan, recognize)
}
func newDestinationPlan(workingDirectory, target string, replace bool) (DestinationPlan, error) {
workingDirectory, err := absoluteCleanPath(workingDirectory)
if err != nil {
return DestinationPlan{}, newDestinationError(DestinationInvalidPath, workingDirectory, err)
@@ -91,7 +103,19 @@ func PlanDestination(workingDirectory, target string, replace bool) (Destination
return DestinationPlan{}, newDestinationError(DestinationInvalidPath, target, err)
}
plan := DestinationPlan{WorkingDirectory: workingDirectory, Target: target, Replace: replace}
return DestinationPlan{WorkingDirectory: workingDirectory, Target: target, Replace: replace}, nil
}
func reauthorizeDestination(plan DestinationPlan) (DestinationPlan, error) {
plan, err := newDestinationPlan(plan.WorkingDirectory, plan.Target, plan.Replace)
if err != nil {
return DestinationPlan{}, err
}
return inspectDestination(plan, nil)
}
func inspectDestination(plan DestinationPlan, recognize func(string) (Manifest, error)) (DestinationPlan, error) {
target := plan.Target
info, err := os.Lstat(target)
if err != nil {
if !errors.Is(err, os.ErrNotExist) {
@@ -117,11 +141,13 @@ func PlanDestination(workingDirectory, target string, replace bool) (Destination
plan.state = destinationEmpty
return plan, nil
}
if !replace {
if !plan.Replace {
return DestinationPlan{}, newDestinationError(DestinationNotEmpty, target, nil)
}
if _, err := RecognizeBundle(target); err != nil {
return DestinationPlan{}, newDestinationError(DestinationUnrecognized, target, err)
if recognize != nil {
if _, err := recognize(target); err != nil {
return DestinationPlan{}, newDestinationError(DestinationUnrecognized, target, err)
}
}
plan.state = destinationBundle
return plan, nil
@@ -188,7 +214,25 @@ func newDestinationError(kind DestinationErrorKind, target string, err error) er
// RecognizeBundle verifies that directory contains exactly one valid current
// comparison bundle. It never follows bundle entries through symlinks.
func RecognizeBundle(directory string) (Manifest, error) {
info, err := os.Lstat(directory)
return recognizeBundle(directory, defaultRecognitionOperations)
}
type recognitionOperations struct {
lstat func(string) (os.FileInfo, error)
readDir func(string) ([]os.DirEntry, error)
open func(string) (io.ReadCloser, error)
}
var defaultRecognitionOperations = recognitionOperations{
lstat: os.Lstat,
readDir: os.ReadDir,
open: func(path string) (io.ReadCloser, error) {
return os.Open(path)
},
}
func recognizeBundle(directory string, operations recognitionOperations) (Manifest, error) {
info, err := operations.lstat(directory)
if err != nil {
return Manifest{}, unrecognizedBundleError("inspect directory", err)
}
@@ -196,11 +240,11 @@ func RecognizeBundle(directory string) (Manifest, error) {
return Manifest{}, unrecognizedBundleError("directory is not a real directory", nil)
}
entries, err := os.ReadDir(directory)
entries, err := operations.readDir(directory)
if err != nil {
return Manifest{}, unrecognizedBundleError("read directory", err)
}
manifestData, err := readBundleFile(directory, ManifestFilename)
manifestData, err := readBundleFile(directory, ManifestFilename, operations)
if err != nil {
return Manifest{}, err
}
@@ -228,12 +272,15 @@ func RecognizeBundle(directory string) (Manifest, error) {
if _, ok := expected[entry.Name()]; !ok {
return Manifest{}, unrecognizedBundleError("directory has an undeclared entry", nil)
}
if _, err := readBundleFile(directory, entry.Name()); err != nil {
if entry.Name() == ManifestFilename || entry.Name() == DataPackageFilename {
continue
}
if err := inspectBundleFile(directory, entry.Name(), operations); err != nil {
return Manifest{}, err
}
}
dataPackage, err := readBundleFile(directory, DataPackageFilename)
dataPackage, err := readBundleFile(directory, DataPackageFilename, operations)
if err != nil {
return Manifest{}, err
}
@@ -243,23 +290,47 @@ func RecognizeBundle(directory string) (Manifest, error) {
return manifest, nil
}
func readBundleFile(directory, name string) ([]byte, error) {
func readBundleFile(directory, name string, operations recognitionOperations) ([]byte, error) {
file, err := openBundleFile(directory, name, operations)
if err != nil {
return nil, err
}
defer file.Close()
data, err := io.ReadAll(file)
if err != nil {
return nil, unrecognizedBundleError("read bundle entry", err)
}
return data, nil
}
func inspectBundleFile(directory, name string, operations recognitionOperations) error {
file, err := openBundleFile(directory, name, operations)
if err != nil {
return err
}
if err := file.Close(); err != nil {
return unrecognizedBundleError("close bundle entry", err)
}
return nil
}
func openBundleFile(directory, name string, operations recognitionOperations) (io.ReadCloser, error) {
if !isArtifactBasename(name) {
return nil, unrecognizedBundleError("bundle entry name is unsafe", nil)
}
filePath := filepath.Join(directory, name)
info, err := os.Lstat(filePath)
info, err := operations.lstat(filePath)
if err != nil {
return nil, unrecognizedBundleError("inspect bundle entry", err)
}
if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() {
return nil, unrecognizedBundleError("bundle entry is not a regular file", nil)
}
data, err := os.ReadFile(filePath)
file, err := operations.open(filePath)
if err != nil {
return nil, unrecognizedBundleError("read bundle entry", err)
}
return data, nil
return file, nil
}
func decodeManifest(data []byte) (Manifest, error) {
@@ -499,6 +570,7 @@ type publishOperations struct {
rename func(string, string) error
removeAll func(string) error
beforeCommit func()
recognize func(string) (Manifest, error)
}
func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, operations publishOperations) (PublicationResult, error) {
@@ -508,6 +580,9 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
if operations.removeAll == nil {
operations.removeAll = os.RemoveAll
}
if operations.recognize == nil {
operations.recognize = RecognizeBundle
}
if err := bundle.Validate(); err != nil {
return PublicationResult{}, fmt.Errorf("validate comparison bundle: %w", err)
}
@@ -518,7 +593,7 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
if err := ctx.Err(); err != nil {
return PublicationResult{}, err
}
plan, err = PlanDestination(plan.WorkingDirectory, plan.Target, plan.Replace)
plan, err = reauthorizeDestination(plan)
if err != nil {
return PublicationResult{}, err
}
@@ -544,7 +619,7 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
return PublicationResult{}, err
}
currentPlan, err := PlanDestination(plan.WorkingDirectory, plan.Target, plan.Replace)
currentPlan, err := reauthorizeDestination(plan)
if err != nil {
return PublicationResult{}, err
}
@@ -569,7 +644,7 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
if err := operations.rename(currentPlan.Target, backupDirectory); err != nil {
return PublicationResult{}, fmt.Errorf("back up comparison destination %q: %w", currentPlan.Target, err)
}
if err := authorizeMovedDestination(currentPlan, backupDirectory); err != nil {
if err := authorizeMovedDestination(currentPlan, backupDirectory, operations.recognize); err != nil {
return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err)
}
if err := ctx.Err(); err != nil {
@@ -600,8 +675,8 @@ func inspectBackupRecovery(backupDirectory string) (BackupRecoveryState, string)
return BackupRecoveryPartial, backupDirectory
}
func authorizeMovedDestination(plan DestinationPlan, backupDirectory string) error {
backupPlan, err := PlanDestination(plan.WorkingDirectory, backupDirectory, plan.Replace)
func authorizeMovedDestination(plan DestinationPlan, backupDirectory string, recognize func(string) (Manifest, error)) error {
backupPlan, err := planDestination(plan.WorkingDirectory, backupDirectory, plan.Replace, recognize)
if err != nil {
return fmt.Errorf("authorize moved comparison destination %q: %w", backupDirectory, err)
}

106
internal/comparison/publish_test.go
View File

@@ -4,6 +4,8 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"runtime"
@@ -222,6 +224,75 @@ func TestPlanDestinationAcceptsRecognizedReplacement(t *testing.T) {
}
}
func TestRecognizeBundleReadsOnlyRequiredArtifactContents(t *testing.T) {
for _, reportCount := range []int{2, 12} {
t.Run(fmt.Sprintf("%d reports", reportCount), func(t *testing.T) {
bundle := testBundleWithReports(t, reportCount)
directory := publishTestBundle(t, bundle)
bytesRead := make(map[string]int)
opens := make(map[string]int)
operations := defaultRecognitionOperations
operations.open = func(path string) (io.ReadCloser, error) {
file, err := os.Open(path)
if err != nil {
return nil, err
}
name := filepath.Base(path)
opens[name]++
return &countingReadCloser{ReadCloser: file, count: func(n int) {
bytesRead[name] += n
}}, nil
}
if _, err := recognizeBundle(directory, operations); err != nil {
t.Fatalf("recognizeBundle() error = %v", err)
}
for _, name := range []string{ManifestFilename, DataPackageFilename} {
data := readFile(t, filepath.Join(directory, name))
if opens[name] != 1 || bytesRead[name] != len(data) {
t.Fatalf("%s opens/bytes = %d/%d, want 1/%d", name, opens[name], bytesRead[name], len(data))
}
}
for _, report := range bundle.Reports {
if opens[report.Path] != 1 || bytesRead[report.Path] != 0 {
t.Fatalf("report %s opens/bytes = %d/%d, want 1/0", report.Path, opens[report.Path], bytesRead[report.Path])
}
}
})
}
}
func TestReplacementUsesEarlyAndFinalRecognition(t *testing.T) {
workingDirectory := t.TempDir()
target := filepath.Join(workingDirectory, "comparison-daily")
bundle := testBundleWithReports(t, 12)
initialPlan, err := PlanDestination(workingDirectory, target, false)
if err != nil {
t.Fatal(err)
}
if _, err := Publish(context.Background(), initialPlan, bundle); err != nil {
t.Fatal(err)
}
recognitions := 0
recognize := func(directory string) (Manifest, error) {
recognitions++
return RecognizeBundle(directory)
}
plan, err := planDestination(workingDirectory, target, true, recognize)
if err != nil {
t.Fatal(err)
}
if _, err := publish(context.Background(), plan, bundle, publishOperations{
rename: os.Rename, removeAll: os.RemoveAll, recognize: recognize,
}); err != nil {
t.Fatal(err)
}
if recognitions != 2 {
t.Fatalf("full bundle recognitions = %d, want early and final authorization", recognitions)
}
}
func TestPublishReauthorizesMovedDestination(t *testing.T) {
tests := []struct {
name string
@@ -995,6 +1066,41 @@ func testBundle() LogicalBundle {
}
}
func testBundleWithReports(t *testing.T, count int) LogicalBundle {
t.Helper()
dataPackage := []byte("report: daily\n")
manifest := validManifest()
manifest.Total, manifest.Succeeded, manifest.Failed = count, count, 0
manifest.Results = make([]Result, count)
reports := make([]BundleReport, count)
for i := range manifest.Results {
position := i + 1
profileID := fmt.Sprintf("weather-%d", position)
path, err := ReportFilename(position, count, profileID)
if err != nil {
t.Fatal(err)
}
manifest.Results[i] = Result{
Position: position, ProfileID: profileID, ModelName: "gpt-5-mini",
Status: StatusSucceeded, ValidationStatus: "passed", ReportPath: path,
}
reports[i] = BundleReport{Position: position, Path: path, Markdown: []byte("# Daily\n")}
}
manifest.DataPackage.SHA256 = SHA256(dataPackage)
return LogicalBundle{Manifest: manifest, DataPackage: dataPackage, Reports: reports}
}
type countingReadCloser struct {
io.ReadCloser
count func(int)
}
func (reader *countingReadCloser) Read(buffer []byte) (int, error) {
n, err := reader.ReadCloser.Read(buffer)
reader.count(n)
return n, err
}
func assertMode(t *testing.T, path string, want os.FileMode) {
t.Helper()
info, err := os.Stat(path)