Scope candidate blob readers to validation

This commit is contained in:
2026-10-09 01:30:30 +00:00
parent 33e10635b6
commit a3e993d95a
13 changed files with 244 additions and 4 deletions

View File

@@ -47,7 +47,13 @@ and checks supported anchors; outside-checkout and submodule observations are
written to the shared output writer. A nil writer keeps library use quiet.
`CandidateTree` gets metadata and bytes from Git objects, reads blob bytes
lazily, and resolves destinations without consulting the worktree or
neighboring files.
neighboring files. Each tree owns one lazy Git blob reader; direct tree users
must close it when finished. The shared validation executor closes its tree on
every return before product checks or build callbacks run. A metadata-only
selection never starts the reader. Per-read contexts cancel an in-flight blob
exchange without tying the reader lifetime to a short-lived caller context.
Until formatting and parsed-result reuse are in place, regular-file bytes stay
cached by object ID for the lifetime of the tree.
The internal Markdown parser uses Goldmark to record inline, reference, and
autolinks, images, titles, and their source use sites, and to build heading

View File

@@ -1,6 +1,6 @@
# Computational Efficiency Implementation Plan
**Status:** Stages 1–4 complete; Stages 5–11 pending.
**Status:** Stages 1–5 complete; Stages 6–11 pending.
## Purpose And Baseline
@@ -316,6 +316,22 @@ preserve workspace/file-size metadata behavior. Re-run content benchmarks with
reader startup and closure included in the content-operation timing. Update
[validation mechanics](../internal/validation.md) for the landed lifecycle.
**Completion:** Complete. `CandidateTree` owns a lazy batch reader and exposes
idempotent cleanup. The shared executor closes it on every post-load return,
before product checks or build callbacks; metadata-only runs leave it unstarted.
Earlier check failures and cancellation remain primary, while an unexpected
reader exit after successful content checks becomes a contextual failure for
the last content-reading check. Direct tree users close their readers. The
temporary object-ID byte cache remains until Stage 6. Validation, candidate,
and root facade tests passed. Content benchmark timings now include reader
startup and close (Go 1.26.8, Git 2.53.0, Linux x86_64 Intel Core i5-7260U,
three repetitions): unique reads took 59.8–77.9 ms loose and 22.8–43.0 ms
packed; unique-file formatting took 114–188 ms loose and 33.7–55.7 ms packed;
repeated-ID reads took 2.18–6.45 ms, repeated-ID formatting took 4.75–6.62 ms,
and the Markdown workload took 15.0–16.9 ms. Compared with the Stage 1 timings,
these measurements reflect the shared Git process and include its lifecycle;
timing varies on the shared machine.
## Stage 6: Release One-Use Source Bytes And Reuse Formatting Results
**Owners:** `internal/validation/tree.go`, `validation.go`, and destination tests.

View File

@@ -43,6 +43,7 @@ func TestResolveDestinationClassifiesCommittedPaths(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
if err := os.Remove(filepath.Join(repository, "docs", "inside")); err != nil {
t.Fatal(err)
}
@@ -102,6 +103,7 @@ func TestResolveDestinationHandlesExpandingCyclesAndRepeatedLinks(t *testing.T)
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
for _, test := range []struct {
destination string
kind DestinationKind
@@ -155,6 +157,7 @@ func TestResolveDestinationIgnoresNeighboringFiles(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
gotWithoutNeighbor, err := tree.ResolveDestination(context.Background(), "docs/page.md", "outside")
if err != nil {
t.Fatal(err)
@@ -195,6 +198,7 @@ func TestResolveDestinationPreservesParentTraversalOrder(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
for _, test := range []struct {
destination string
kind DestinationKind
@@ -253,6 +257,7 @@ func TestResolveDestinationMapsAbsolutePathsIntoCandidate(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
if err := os.Remove(filepath.Join(repository, "README.md")); err != nil {
t.Fatal(err)
}

View File

@@ -135,6 +135,7 @@ func TestCheckFileSizeUsesCommittedBlobSizes(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkFileSize(tree, &FileSizeSettings{LimitBytes: int64Pointer(3), Exclusions: []string{"link"}})
if err != nil {
@@ -170,6 +171,7 @@ func TestCheckFileSizeUsesOnlyCandidateFilesIndividually(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
if err := checkFileSize(tree, &FileSizeSettings{LimitBytes: int64Pointer(10)}); err != nil {
t.Fatalf("checkFileSize() error = %v, want current individual files to pass", err)
}

View File

@@ -36,6 +36,7 @@ func TestCheckGoFormatUsesCapturedRegularGoBlobs(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkGoFormat(context.Background(), tree)
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat {
@@ -70,6 +71,7 @@ func TestCheckGoFormatPreservesParserAndInspectionCauses(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkGoFormat(context.Background(), tree)
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat || failure.Cause == nil {
@@ -90,6 +92,7 @@ func TestCheckGoFormatPreservesParserAndInspectionCauses(t *testing.T) {
byPath: map[string]int{"missing.go": 0},
blobs: make(map[string][]byte),
}
cleanupCandidateTree(t, tree)
err := checkGoFormat(context.Background(), tree)
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat || failure.Cause == nil {

View File

@@ -21,6 +21,7 @@ func TestCheckLocalReplacementsParsesCommittedModuleDeclarations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkLocalReplacements(context.Background(), tree)
failure, ok := err.(*Failure)
@@ -61,6 +62,7 @@ func TestCheckLocalReplacementsFollowsCommittedModuleSymlinks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkLocalReplacements(context.Background(), tree)
failure, ok := err.(*Failure)
if !ok || failure.Check != LocalReplacements || len(failure.Findings) != 1 {
@@ -82,6 +84,7 @@ func TestCheckLocalReplacementsReportsUnavailableSymlinkContents(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkLocalReplacements(context.Background(), tree)
failure, ok := err.(*Failure)
if !ok || failure.Check != LocalReplacements || failure.Cause == nil {
@@ -100,6 +103,7 @@ func TestCheckLocalReplacementsPreservesParseCause(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
err = checkLocalReplacements(context.Background(), tree)
failure, ok := err.(*Failure)
if !ok || failure.Check != LocalReplacements || failure.Cause == nil || !strings.Contains(failure.Cause.Error(), "parse committed go.mod") {

View File

@@ -289,6 +289,7 @@ func candidateTreeAtHead(t *testing.T, repository string) *CandidateTree {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
return tree
}

View File

@@ -99,6 +99,7 @@ func TestMarkdownCacheKeepsParsedDocumentsPerCandidatePath(t *testing.T) {
if err != nil {
t.Fatal(err)
}
cleanupCandidateTree(t, tree)
cache := newMarkdownCache(tree)
first, err := cache.document(context.Background(), "guide.md")
if err != nil {

View File

@@ -54,6 +54,9 @@ func BenchmarkPerformanceValidation(b *testing.B) {
if len(tree.Entries()) != len(fixture.paths) {
b.Fatalf("tree entries = %d, want %d", len(tree.Entries()), len(fixture.paths))
}
if err := tree.Close(); err != nil {
b.Fatal(err)
}
runtime.KeepAlive(tree)
}
})
@@ -77,6 +80,9 @@ func BenchmarkPerformanceValidation(b *testing.B) {
if bytesRead != fixture.totalBytes {
b.Fatalf("read %d bytes, want %d", bytesRead, fixture.totalBytes)
}
if err := tree.Close(); err != nil {
b.Fatal(err)
}
runtime.KeepAlive(tree)
}
runtime.KeepAlive(trees)
@@ -92,6 +98,9 @@ func BenchmarkPerformanceValidation(b *testing.B) {
if err := checkGoFormat(context.Background(), tree); err != nil {
b.Fatal(err)
}
if err := tree.Close(); err != nil {
b.Fatal(err)
}
runtime.KeepAlive(tree)
}
runtime.KeepAlive(trees)
@@ -109,6 +118,9 @@ func BenchmarkPerformanceValidation(b *testing.B) {
if err := checkMarkdownLinks(context.Background(), tree, false, io.Discard); err != nil {
b.Fatal(err)
}
if err := tree.Close(); err != nil {
b.Fatal(err)
}
runtime.KeepAlive(tree)
}
runtime.KeepAlive(trees)
@@ -121,6 +133,7 @@ func BenchmarkPerformanceRetainedContent(b *testing.B) {
if err != nil {
b.Fatal(err)
}
b.Cleanup(func() { _ = tree.Close() })
cache := newMarkdownCache(tree)
var markdownPaths []string
for _, entry := range tree.Entries() {
@@ -149,6 +162,9 @@ func BenchmarkPerformanceRetainedContent(b *testing.B) {
}
}
b.StopTimer()
if err := tree.Close(); err != nil {
b.Fatal(err)
}
runtime.GC()
var after runtime.MemStats
runtime.ReadMemStats(&after)
@@ -209,12 +225,16 @@ func freshPerformanceTrees(tb testing.TB, fixture performanceTree, count int) []
if err != nil {
tb.Fatal(err)
}
cleanupCandidateTree(tb, base)
trees := make([]*CandidateTree, count)
for index := range trees {
tree := *base
tree.blobs = make(map[string][]byte)
trees[index] = &tree
}
if err := base.Close(); err != nil {
tb.Fatal(err)
}
runtime.KeepAlive(base)
return trees
}
@@ -270,6 +290,9 @@ func verifyPerformanceTree(tb testing.TB, fixture performanceTree, repeated, pac
tb.Fatal("Markdown link fixture does not contain the expected repeated targets")
}
}
if err := tree.Close(); err != nil {
tb.Fatal(err)
}
objects := gitTree(tb, fixture.repository, "count-objects", "-v")
loose, packedCount := performanceObjectCounts(tb, objects)

View File

@@ -18,6 +18,9 @@ type CandidateTree struct {
byPath map[string]int
directories map[string]struct{}
blobs map[string][]byte
blobReader *git.BlobReader
closed bool
closeErr error
}
// ReadCandidateTree enumerates the captured commit without reading from the
@@ -52,6 +55,9 @@ func (tree *CandidateTree) Entries() []git.TreeEntry {
// ReadFile returns the committed blob bytes for path. Contents are cached by
// object ID for later reads from this tree.
func (tree *CandidateTree) ReadFile(ctx context.Context, path string) ([]byte, error) {
if tree.closed {
return nil, fmt.Errorf("read committed path %q: candidate tree is closed", path)
}
index, ok := tree.byPath[path]
if !ok {
return nil, fmt.Errorf("committed path %q does not exist", path)
@@ -63,14 +69,36 @@ func (tree *CandidateTree) ReadFile(ctx context.Context, path string) ([]byte, e
if contents, ok := tree.blobs[entry.ObjectID]; ok {
return append([]byte(nil), contents...), nil
}
contents, err := git.ReadBlob(ctx, tree.repositoryDir, entry.ObjectID)
if tree.blobReader == nil {
// The reader belongs to the tree and can outlive an individual read
// context. Each Read call still applies its own cancellation to the
// in-flight exchange.
tree.blobReader = git.NewBlobReader(context.Background(), tree.repositoryDir)
}
contents, err := tree.blobReader.Read(ctx, entry.ObjectID)
if err != nil {
return nil, fmt.Errorf("read committed path %q: %w", path, err)
}
if tree.blobs == nil {
tree.blobs = make(map[string][]byte)
}
tree.blobs[entry.ObjectID] = append([]byte(nil), contents...)
return contents, nil
}
// Close closes the scoped local Git blob reader, if content was requested.
// Metadata-only trees have no child process to close.
func (tree *CandidateTree) Close() error {
if tree.closed {
return tree.closeErr
}
tree.closed = true
if tree.blobReader != nil {
tree.closeErr = tree.blobReader.Close()
}
return tree.closeErr
}
// Entry returns the committed metadata for path when present.
func (tree *CandidateTree) Entry(path string) (git.TreeEntry, bool) {
index, ok := tree.byPath[path]

View File

@@ -0,0 +1,133 @@
package validation
import (
"context"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"
)
func TestRunAttributesUnexpectedReaderCloseFailureToLastContentCheck(t *testing.T) {
repository := newTreeRepository(t)
writeTreeFile(t, repository, "go.mod", []byte("module fixture.example/app\n\ngo 1.25.0\n"))
writeTreeFile(t, repository, "main.go", []byte("package app\n\nconst value = 1\n"))
commit := commitCandidateTree(t, repository)
installLifecycleTools(t, `while IFS= read -r object; do
size=$("$RELEASEKIT_REAL_GIT" -C "$PWD" cat-file -s "$object")
printf '%s blob %s\n' "$object" "$size"
"$RELEASEKIT_REAL_GIT" -C "$PWD" cat-file blob "$object"
printf '\n'
done
printf 'batch process failed during cleanup\n' >&2
exit 23`)
err := Run(context.Background(), lifecycleRequest(repository, commit))
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat {
t.Fatalf("Run() error = %v, want cleanup failure attributed to GoFormat", err)
}
var exitErr *exec.ExitError
if !errors.As(failure.Cause, &exitErr) || exitErr.ExitCode() != 23 {
t.Fatalf("cleanup cause = %v, want Git child exit code 23", failure.Cause)
}
if !strings.Contains(failure.Cause.Error(), "batch process failed during cleanup") {
t.Fatalf("cleanup cause = %v, want captured Git stderr", failure.Cause)
}
}
func TestRunPreservesCheckFailureDuringReaderCleanup(t *testing.T) {
repository := newTreeRepository(t)
writeTreeFile(t, repository, "go.mod", []byte("module fixture.example/app\n\ngo 1.25.0\n"))
writeTreeFile(t, repository, "broken.go", []byte("package app\nfunc ( {\n"))
commit := commitCandidateTree(t, repository)
installLifecycleTools(t, `while IFS= read -r object; do
size=$("$RELEASEKIT_REAL_GIT" -C "$PWD" cat-file -s "$object")
printf '%s blob %s\n' "$object" "$size"
"$RELEASEKIT_REAL_GIT" -C "$PWD" cat-file blob "$object"
printf '\n'
done
printf 'secondary cleanup failure\n' >&2
exit 24`)
err := Run(context.Background(), lifecycleRequest(repository, commit))
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat || failure.Cause == nil {
t.Fatalf("Run() error = %v, want original GoFormat check failure", err)
}
if len(failure.Findings) != 1 || failure.Findings[0].Path != "broken.go" {
t.Fatalf("Run() findings = %#v, want original broken.go finding", failure.Findings)
}
if strings.Contains(failure.Cause.Error(), "secondary cleanup failure") {
t.Fatalf("Run() replaced the check cause with cleanup output: %v", failure.Cause)
}
}
func TestRunPreservesCancellationDuringReaderCleanup(t *testing.T) {
repository := newTreeRepository(t)
writeTreeFile(t, repository, "go.mod", []byte("module fixture.example/app\n\ngo 1.25.0\n"))
writeTreeFile(t, repository, "main.go", []byte("package app\n\nconst value = 1\n"))
commit := commitCandidateTree(t, repository)
ready := filepath.Join(t.TempDir(), "reader-ready")
installLifecycleTools(t, `IFS= read -r object
: > "$RELEASEKIT_GIT_READER_READY"
exec sleep 60`)
t.Setenv("RELEASEKIT_GIT_READER_READY", ready)
ctx, cancel := context.WithCancel(t.Context())
result := make(chan error, 1)
go func() { result <- Run(ctx, lifecycleRequest(repository, commit)) }()
waitForLifecycleFile(t, ready)
cancel()
select {
case err := <-result:
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat || !errors.Is(failure.Cause, context.Canceled) {
t.Fatalf("Run() error = %v, want GoFormat cancellation cause", err)
}
case <-time.After(5 * time.Second):
t.Fatal("Run() did not return after candidate reader cancellation")
}
}
func lifecycleRequest(repository, commit string) Request {
return Request{
RepositoryDir: repository,
Commit: commit,
Settings: Settings{GoFormat: true},
}
}
func installLifecycleTools(t *testing.T, batchScript string) {
t.Helper()
realGit, err := exec.LookPath("git")
if err != nil {
t.Fatal(err)
}
installGoShim(t, "", false)
t.Setenv("RELEASEKIT_REAL_GIT", realGit)
directory := t.TempDir()
gitWrapper := filepath.Join(directory, "git")
script := "#!/bin/sh\nset -eu\nif [ \"${1-}\" = cat-file ] && [ \"${2-}\" = --batch ]; then\n" + batchScript + "\nfi\nexec \"$RELEASEKIT_REAL_GIT\" \"$@\"\n"
if err := os.WriteFile(gitWrapper, []byte(script), 0o700); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", directory+string(os.PathListSeparator)+os.Getenv("PATH"))
}
func waitForLifecycleFile(t *testing.T, path string) {
t.Helper()
deadline := time.Now().Add(5 * time.Second)
for time.Now().Before(deadline) {
if _, err := os.Stat(path); err == nil {
return
} else if !errors.Is(err, os.ErrNotExist) {
t.Fatal(fmt.Errorf("inspect reader readiness signal: %w", err))
}
time.Sleep(5 * time.Millisecond)
}
t.Fatal("candidate blob reader did not reach its blocking exchange")
}

View File

@@ -38,6 +38,7 @@ func TestReadCandidateTreeUsesCapturedCommitAndEnumeratesGitEntries(t *testing.T
if err != nil {
t.Fatalf("ReadCandidateTree() error = %v", err)
}
cleanupCandidateTree(t, tree)
entries := make(map[string]gitTreeEntry, len(tree.Entries()))
for _, entry := range tree.Entries() {
entries[entry.Path] = gitTreeEntry{mode: entry.Mode, kind: entry.Type, size: entry.Size, hasSize: entry.HasSize}
@@ -77,6 +78,11 @@ func TestReadCandidateTreeUsesCapturedCommitAndEnumeratesGitEntries(t *testing.T
}
}
func cleanupCandidateTree(tb testing.TB, tree *CandidateTree) {
tb.Helper()
tb.Cleanup(func() { _ = tree.Close() })
}
func TestReadCandidateTreePreservesGitProcessError(t *testing.T) {
repository := newTreeRepository(t)
_, err := ReadCandidateTree(context.Background(), repository, strings.Repeat("f", 40))

View File

@@ -63,7 +63,7 @@ func (request Request) ReadCandidateTree(ctx context.Context) (*CandidateTree, e
}
// Run executes shared validation for the captured candidate.
func Run(ctx context.Context, request Request) error {
func Run(ctx context.Context, request Request) (result error) {
if request.Settings.FileSize != nil && request.Settings.FileSize.LimitBytes != nil && *request.Settings.FileSize.LimitBytes < 0 {
return &Failure{
Check: FileSize,
@@ -136,7 +136,17 @@ func Run(ctx context.Context, request Request) error {
}
return &Failure{Check: check, Cause: err}
}
lastContentCheck := Check("")
defer func() {
if closeErr := tree.Close(); closeErr != nil && result == nil && lastContentCheck != "" {
result = &Failure{
Check: lastContentCheck,
Cause: fmt.Errorf("close candidate Git blob reader: %w", closeErr),
}
}
}()
if request.Settings.GoFormat {
lastContentCheck = GoFormat
if err := checkGoFormat(ctx, tree); err != nil {
return err
}
@@ -147,6 +157,7 @@ func Run(ctx context.Context, request Request) error {
}
}
if request.Settings.ModuleDistribution.LocalReplacements {
lastContentCheck = LocalReplacements
if err := checkLocalReplacements(ctx, tree); err != nil {
return err
}
@@ -157,6 +168,7 @@ func Run(ctx context.Context, request Request) error {
}
}
if request.Settings.Markdown != nil {
lastContentCheck = MarkdownLinks
if err := checkMarkdownLinks(ctx, tree, request.Settings.Markdown.EnforceCheckoutBoundary, request.Output); err != nil {
return err
}