Reuse local asset verification during delivery uploads

delivery_reads_linux_test.go

@@ -0,0 +1,66 @@
+package releasekit_test
+
+import (
+	"encoding/binary"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"syscall"
+	"testing"
+
+	releasekit "gitea.maximumdirect.net/eric/ReleaseKit"
+)
+
+func TestDeliverReadsLocalBinaryOnceForVerificationAndOnceForUpload(t *testing.T) {
+	repository, build, expected := newDeliveryBenchmarkFixture(t)
+	watcher, err := syscall.InotifyInit1(syscall.IN_NONBLOCK | syscall.IN_CLOEXEC)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer syscall.Close(watcher)
+	// Close events separate successive opens so inotify cannot coalesce them.
+	if _, err := syscall.InotifyAddWatch(watcher, build.Artifacts[0].Path, syscall.IN_OPEN|syscall.IN_CLOSE_NOWRITE); err != nil {
+		t.Fatal(err)
+	}
+	state := &deliveryBenchmarkServer{commit: build.CandidateCommit, assets: expected}
+	state.reset(false)
+	server := httptest.NewServer(http.HandlerFunc(state.serveHTTP))
+	defer server.Close()
+	state.baseURL = server.URL
+	result, err := releasekit.Deliver(t.Context(), releasekit.DeliveryRequest{
+		RepositoryDir: repository, Product: "Sample App", Version: build.Version,
+		Destination: facadeDeliveryDestination(server.URL), Prebuilt: &build,
+	})
+	if err != nil || result.State != releasekit.DeliveryComplete || len(result.Uploaded) != len(expected) {
+		t.Fatalf("delivery = %#v, error = %v", result, err)
+	}
+	state.verifyUploads(t)
+	opens := 0
+	var events [256]byte
+	for {
+		n, err := syscall.Read(watcher, events[:])
+		if errors.Is(err, syscall.EAGAIN) {
+			break
+		}
+		if errors.Is(err, syscall.EINTR) {
+			continue
+		}
+		if err != nil {
+			t.Fatal(err)
+		}
+		for offset := 0; offset < n; {
+			mask := binary.NativeEndian.Uint32(events[offset+4:])
+			if mask&syscall.IN_Q_OVERFLOW != 0 {
+				t.Fatal("artifact event queue overflowed")
+			}
+			if mask&syscall.IN_OPEN != 0 {
+				opens++
+			}
+			nameLength := binary.NativeEndian.Uint32(events[offset+12:])
+			offset += syscall.SizeofInotifyEvent + int(nameLength)
+		}
+	}
+	if opens != 2 {
+		t.Fatalf("local binary opened %d times; want one verification read and one upload read", opens)
+	}
+}

docs/internal/overview.md

@@ -74,6 +74,12 @@ it does not consult neighboring files or traverse submodule checkouts.
 See the [shared validation guide](validation.md) for checker, Go command, and
 candidate-file integration mechanics.
 
+Delivery verifies all local binary sizes and digests before hosted requests.
+Its Gitea upload path reuses that verification and streams missing files once
+for transport. Independent calls to the internal client's `UploadMissing`
+verify missing local files before writing. Both paths share remote inspection,
+conflict handling, and reconciliation.
+
 ## Capability owners and behavioral evidence
 
 The following tests are representative evidence from the current offline suite,

docs/roadmap/remediation.md

@@ -1,6 +1,6 @@
 # Remediation: module release implementation review
 
-Status: open; issues 1–12 are complete, and issues 13–15 require remediation.
+Status: open; issues 1–13 are complete, and issues 14–15 require remediation.
 The initial review examined `90559fe`, compared with planning baseline `dbfae9e`.
 The follow-up repository audit examined `6b0b8ce`, including those corrections.
 
@@ -462,7 +462,17 @@ differences, ordinary aliases, cycles, and outside-checkout failures.
 
 ## Issue 13: Reuse local asset verification through the complete delivery path (P2)
 
-Status: open. Category: incomplete verification ownership and redundant I/O.
+Status: complete. Category: incomplete verification ownership and redundant I/O.
+
+**Completion:** Assembled delivery reuses its preflight verification through the
+internal upload path; standalone internal uploads retain local verification.
+A Linux filesystem-event regression observes one verification read and one
+upload read across public delivery and validates uploaded contents. It failed
+with three reads before the correction. Delivery/Gitea suites pass offline,
+including tampering, caller ownership, matching attachments, and partial retries.
+The complete-delivery benchmark now supplies current module identity. Its 32 MiB
+fresh-upload case measured about 352 ms before and 254 ms afterward on this host;
+these illustrative measurements introduce no timing gate.
 
 **Evidence:** [Delivery](../../internal/delivery/delivery.go) calls
 `VerifyLocalAssets` before hosted requests. After release and attachment

internal/delivery/delivery.go

@@ -183,7 +183,7 @@ func Deliver(ctx context.Context, request Request) (Result, error) {
 		result.Uncertain = assetIdentities(assets)
 		return result, err
 	}
-	uploadResult, uploadErr := (gitea.Client{}).UploadMissing(ctx, request.Destination, releaseResult.Release.ID, inspection)
+	uploadResult, uploadErr := (gitea.Client{}).UploadVerifiedMissing(ctx, request.Destination, releaseResult.Release.ID, inspection)
 	result.Matching = uploadResult.Matching
 	result.Uploaded = uploadResult.Uploaded
 	result.Pending = uploadResult.Pending

internal/gitea/upload.go

@@ -68,6 +68,17 @@ func VerifyLocalAssets(assets []ExpectedAsset) error {
 // before the first write if any declared existing asset conflicts or cannot be
 // verified, and never replaces an existing attachment.
 func (c Client) UploadMissing(ctx context.Context, destination Destination, releaseID int64, inspection Inspection) (UploadResult, error) {
+	return c.uploadMissing(ctx, destination, releaseID, inspection, true)
+}
+
+// UploadVerifiedMissing uploads missing assets whose local contents have already
+// passed VerifyLocalAssets. It reuses that verification while retaining remote
+// inspection, conflict handling, and upload reconciliation.
+func (c Client) UploadVerifiedMissing(ctx context.Context, destination Destination, releaseID int64, inspection Inspection) (UploadResult, error) {
+	return c.uploadMissing(ctx, destination, releaseID, inspection, false)
+}
+
+func (c Client) uploadMissing(ctx context.Context, destination Destination, releaseID int64, inspection Inspection, verifyLocal bool) (UploadResult, error) {
 	if releaseID <= 0 {
 		return UploadResult{State: DeliveryPartial}, failure(FailureInvalid, "upload release assets", 0, errors.New("release ID must be positive"))
 	}
@@ -127,12 +138,14 @@ func (c Client) UploadMissing(ctx context.Context, destination Destination, rele
 			missing[observation.Expected.Filename] = observation.Expected
 		}
 	}
-	for _, item := range expected {
-		if _, needsUpload := missing[item.Filename]; !needsUpload {
-			continue
-		}
-		if err := verifyLocalAsset(item); err != nil {
-			return result, err
+	if verifyLocal {
+		for _, item := range expected {
+			if _, needsUpload := missing[item.Filename]; !needsUpload {
+				continue
+			}
+			if err := verifyLocalAsset(item); err != nil {
+				return result, err
+			}
 		}
 	}
 

internal/gitea/upload_test.go

@@ -15,6 +15,31 @@ import (
 	"testing"
 )
 
+func TestUploadMissingVerifiesEveryMissingLocalFileBeforeWriting(t *testing.T) {
+	fixture := &uploadFixture{files: make(map[string][]byte)}
+	server := httptest.NewServer(fixture)
+	defer server.Close()
+	client := testClient(server)
+	destination := testDestination(server.URL)
+	assets := localAssets(t, []string{"alpha.bin", "beta.bin"})
+	inspection, err := client.InspectAssets(t.Context(), destination, 10, assets)
+	if err != nil {
+		t.Fatal(err)
+	}
+	contents := readExpectedFile(t, assets[1])
+	contents[0] ^= 0xff
+	if err := os.WriteFile(assets[1].Path, contents, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	result, err := client.UploadMissing(t.Context(), destination, 10, inspection)
+	if err == nil || result.State != DeliveryPartial {
+		t.Fatalf("upload = %#v, error = %v; want local verification failure", result, err)
+	}
+	if posts := fixture.postNames(); len(posts) != 0 {
+		t.Fatalf("uploaded %v before rejecting local content", posts)
+	}
+}
+
 func TestUploadMissingPartialFailureThenRerunOnlyMissingAssets(t *testing.T) {
 	fixture := &uploadFixture{files: make(map[string][]byte), failName: "beta.bin"}
 	server := httptest.NewServer(fixture)

performance_delivery_test.go

@@ -102,7 +102,7 @@ func BenchmarkPerformanceFacadeDeliver(b *testing.B) {
 	})
 }
 
-func newDeliveryBenchmarkFixture(b *testing.B) (string, releasekit.BuildResult, []deliveryBenchmarkAsset) {
+func newDeliveryBenchmarkFixture(b testing.TB) (string, releasekit.BuildResult, []deliveryBenchmarkAsset) {
 	b.Helper()
 	repository := filepath.Join(b.TempDir(), "repository")
 	if err := os.MkdirAll(repository, 0o755); err != nil {
@@ -111,6 +111,7 @@ func newDeliveryBenchmarkFixture(b *testing.B) (string, releasekit.BuildResult,
 	gitRun(b, repository, "init", "-b", "main")
 	gitRun(b, repository, "config", "user.name", "ReleaseKit Benchmark")
 	gitRun(b, repository, "config", "user.email", "benchmark@example.invalid")
+	writeFile(b, repository, "go.mod", "module example.com/deliveryfixture\n\ngo 1.26.0\n")
 	writeFile(b, repository, "docs/releases/v1.2.3.md", "# Sample App v1.2.3\n\nDelivery fixture.\n")
 	gitRun(b, repository, "add", ".")
 	gitRun(b, repository, "commit", "-m", "prepare delivery benchmark")
@@ -129,6 +130,7 @@ func newDeliveryBenchmarkFixture(b *testing.B) (string, releasekit.BuildResult,
 	targets := []releasekit.Target{{OS: "linux", Arch: "amd64"}, {OS: "windows", Arch: "amd64"}}
 	build := releasekit.BuildResult{
 		RepositoryDir: repository, CandidateCommit: gitText(b, repository, "rev-parse", "HEAD"),
+		ModuleDir: repository, ModulePath: "example.com/deliveryfixture", Tag: "v1.2.3",
 		SourceVerified: true, Version: "v1.2.3", MainPackage: "./cmd/app",
 		BinaryName: "releasekit", Targets: targets, OutputDir: outputDir,
 	}
@@ -173,7 +175,7 @@ func (state *deliveryBenchmarkServer) reset(populated bool) {
 	}
 }
 
-func (state *deliveryBenchmarkServer) verifyUploads(b *testing.B) {
+func (state *deliveryBenchmarkServer) verifyUploads(b testing.TB) {
 	b.Helper()
 	state.mu.Lock()
 	defer state.mu.Unlock()