From 7d92c477c211964a66c3f67e03bbe2ae6c025f88 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 03:07:13 +0000 Subject: [PATCH] Attribute reader shutdown failures to checks that read content --- docs/roadmap/remediation.md | 8 ++++- internal/validation/tree.go | 2 ++ internal/validation/tree_lifecycle_test.go | 34 +++++++++++++++------- internal/validation/validation.go | 15 ++++++++-- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 74d68fa..eff98cf 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Performance Implementation Remediation -**Status:** Remediation in progress; R1 complete. +**Status:** Remediation in progress; R1–R2 complete. **Review baseline:** df064bd, reviewed on 2026-10-09 against the [performance target](performance.md) and @@ -90,6 +90,12 @@ tracking remains internal implementation bookkeeping. later selected check that has no applicable files. Verify the earlier reader user receives the cleanup cause and the child exit error remains discoverable. +**Completion:** Complete. Internal blob-read bookkeeping now identifies the last +check that actually used the reader. The extended close-failure fixture preserves +Go-format attribution when Markdown has no files, including the child exit code +and stderr. The regression failed before the fix and passes afterward; validation +race tests and compiled public API and dependency checks passed. + ## R3: Qualify Output Allocation Measurements By Their Workload **Priority:** Low. diff --git a/internal/validation/tree.go b/internal/validation/tree.go index b904687..e69cf75 100644 --- a/internal/validation/tree.go +++ b/internal/validation/tree.go @@ -19,6 +19,7 @@ type CandidateTree struct { byPath map[string]int directories map[string]struct{} blobReader *git.BlobReader + blobReads uint64 symlinkTargets map[string]string closed bool closeErr error @@ -113,6 +114,7 @@ func (tree *CandidateTree) readBlob(ctx context.Context, entry git.TreeEntry) ([ } tree.blobReader = git.NewBlobReader(operationContext, tree.repositoryDir) } + tree.blobReads++ contents, err := tree.blobReader.Read(ctx, entry.ObjectID) if err != nil { return nil, fmt.Errorf("read committed path %q: %w", entry.Path, err) diff --git a/internal/validation/tree_lifecycle_test.go b/internal/validation/tree_lifecycle_test.go index 9647ca9..112723a 100644 --- a/internal/validation/tree_lifecycle_test.go +++ b/internal/validation/tree_lifecycle_test.go @@ -28,17 +28,29 @@ 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) + for _, testCase := range []struct { + name string + markdown *MarkdownSettings + }{ + {name: "formatting only"}, + {name: "later Markdown check without files", markdown: &MarkdownSettings{}}, + } { + t.Run(testCase.name, func(t *testing.T) { + request := lifecycleRequest(repository, commit) + request.Settings.Markdown = testCase.markdown + err := Run(context.Background(), request) + 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) + } + }) } } diff --git a/internal/validation/validation.go b/internal/validation/validation.go index 001e522..4ad600f 100644 --- a/internal/validation/validation.go +++ b/internal/validation/validation.go @@ -146,10 +146,13 @@ func Run(ctx context.Context, request Request) (result error) { } }() if request.Settings.GoFormat { - lastContentCheck = GoFormat + readsBefore := tree.blobReads if err := checkGoFormat(ctx, tree); err != nil { return err } + if tree.blobReads != readsBefore { + lastContentCheck = GoFormat + } } if request.Settings.ModuleDistribution.WorkspaceFiles { if err := checkWorkspaceFiles(tree); err != nil { @@ -157,10 +160,13 @@ func Run(ctx context.Context, request Request) (result error) { } } if request.Settings.ModuleDistribution.LocalReplacements { - lastContentCheck = LocalReplacements + readsBefore := tree.blobReads if err := checkLocalReplacements(ctx, tree); err != nil { return err } + if tree.blobReads != readsBefore { + lastContentCheck = LocalReplacements + } } if request.Settings.FileSize != nil { if err := checkFileSize(tree, request.Settings.FileSize); err != nil { @@ -168,10 +174,13 @@ func Run(ctx context.Context, request Request) (result error) { } } if request.Settings.Markdown != nil { - lastContentCheck = MarkdownLinks + readsBefore := tree.blobReads if err := checkMarkdownLinks(ctx, tree, request.Settings.Markdown.EnforceCheckoutBoundary, request.Output); err != nil { return err } + if tree.blobReads != readsBefore { + lastContentCheck = MarkdownLinks + } } } return nil