Preserve asset inspection error causes through delivery

delivery_causes_test.go

@@ -0,0 +1,63 @@
+package releasekit_test
+
+import (
+	"context"
+	"errors"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"runtime"
+	"strings"
+	"testing"
+
+	releasekit "go.maximumdirect.net/releasekit"
+)
+
+func TestDeliveryPreservesAssetInspectionCauses(t *testing.T) {
+	repository := newTaggedDeliveryConsumer(t)
+	prebuilt, err := releasekit.Build(context.Background(), releasekit.BuildRequest{
+		RepositoryDir: repository, Version: "v1.2.3", OutputDir: t.TempDir(),
+		Spec: releasekit.BuildSpec{MainPackage: "./cmd/app", BinaryName: "fixture", Targets: []releasekit.Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	filename := prebuilt.Artifacts[0].Filename
+	for _, cause := range []error{context.Canceled, io.ErrUnexpectedEOF} {
+		t.Run(cause.Error(), func(t *testing.T) {
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+			state := &facadeDeliveryServer{data: facadeDeliveryData{
+				t: t, commit: prebuilt.CandidateCommit, tag: prebuilt.Tag, version: prebuilt.Version,
+				noteBody: "# Fixture v1.2.3\n\nDelivery fixture.\n",
+				pages:    map[string]bool{prebuilt.Tag: true}, assets: map[string][]byte{filename: []byte("data")},
+			}}
+			server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if strings.HasPrefix(r.URL.Path, "/files/") {
+					if cause == context.Canceled {
+						cancel()
+						<-r.Context().Done()
+						return
+					}
+					w.Header().Set("Content-Length", "100")
+					_, _ = w.Write([]byte("truncated"))
+					return
+				}
+				state.serveHTTP(w, r)
+			}))
+			state.data.url = "http://" + server.Listener.Addr().String()
+			server.Start()
+			defer server.Close()
+			result, err := releasekit.Deliver(ctx, releasekit.DeliveryRequest{
+				RepositoryDir: repository, Product: "Fixture", Version: "v1.2.3",
+				Destination: facadeDeliveryDestination(server.URL), Prebuilt: &prebuilt,
+			})
+			if !errors.Is(err, cause) || !strings.Contains(err.Error(), filename) {
+				t.Fatalf("Deliver() error = %v, want cause %v and asset context", err, cause)
+			}
+			if result.State != releasekit.DeliveryNotAttempted || len(result.Uncertain) != 1 || result.Uncertain[0].Filename != filename || state.snapshot().writes != 0 {
+				t.Fatalf("result = %#v, want read-only uncertain asset without write", result)
+			}
+		})
+	}
+}

docs/roadmap/remediation.md

@@ -1,13 +1,14 @@
 # Remediation: forge boundary and credential preflight review
 
-Status: remediation in progress; completion is recorded per issue below.
+Status: complete; all eight issues are addressed, with completion recorded below.
 
 ## Review baseline and scope
 
 Reviewed `cbdba90`, including the implementation commits after `9d6952a`,
 against the [feature roadmap](forge-boundary.md),
 [implementation plan](implementation.md), and the user's selected scope.
-This document records work identified by the review; it does not implement it.
+This document records review findings and their remediation. Findings describe
+the reviewed revisions; completion notes record their corrections.
 
 A follow-up integration audit reviewed `72de187`. Its production source is
 unchanged from `cbdba90`. That audit traced the new boundary and preflight
@@ -338,7 +339,14 @@ successful upload coordination can avoid its quadratic bookkeeping term.
 
 ## 8. Preserve asset-inspection failure causes during aggregation — P2
 
-Status: open.
+Status: complete.
+
+**Completed:** Joined per-asset inspection causes with filename context in
+unverifiable and conflict summaries, retaining conflict precedence and existing
+states/categories. Added focused multiple-cause coverage and public API
+regressions for cancellation and truncated downloads. Both regressions detect
+the original defect and pass with the fix. Full offline tests, focused race
+tests, and the API/dependency check passed; final verification is recorded below.
 
 **Evidence:** `inspectRemoteAssets` and `uploadMissingAssets` in
 [shared asset delivery](../../internal/delivery/remote_assets.go), and the
@@ -438,3 +446,20 @@ finding 8; it was removed after the check. Findings 6 and 7 follow from
 source-level operation counts. No benchmarks
 or real-world performance measurements were run, and no production code or
 permanent tests changed.
+
+### Remediation verification
+
+All eight issues were addressed sequentially and marked complete with their
+implementation or documentation changes. Final checks used Go 1.26.10 on
+linux/amd64 with `GOTOOLCHAIN=local`, `GOPROXY=off`, and `GOSUMDB=off`:
+
+- `go test ./...` passed.
+- `go run ./internal/apicheck` passed.
+- `go test -race ./internal/delivery ./internal/gitea` passed.
+- Focused public API and CLI regressions also passed with the race detector:
+  invalid destinations, CI module/tag mismatch, phase-specific statuses, and
+  asset-inspection cause preservation.
+- A temporary source overlay verified that the new aggregation/public API
+  cause-preservation regressions fail against the previous implementation.
+- Local links and heading fragments across changed Markdown files resolved;
+  whitespace checks passed. No benchmark measurements were needed.

internal/delivery/remote_assets.go

@@ -124,6 +124,7 @@ func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHand
 	result = UploadResult{State: Partial}
 	preflightConflict := false
 	preflightUnverifiable := false
+	var inspectionErrors []error
 	var missing []artifact.ExpectedAsset
 	pending := make(map[string]struct{}, len(expected))
 	// Keep the initial observation order and compact pending identities once,
@@ -152,17 +153,20 @@ func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHand
 		case AssetUnverifiable:
 			preflightUnverifiable = true
 			result.Uncertain = append(result.Uncertain, item)
+			if observation.Err != nil {
+				inspectionErrors = append(inspectionErrors, fmt.Errorf("verify existing attachment %q: %w", item.Filename, observation.Err))
+			}
 		default:
 			return result, &forge.Failure{Kind: forge.FailureInvalid, Err: fmt.Errorf("asset %q has no inspection result", observation.Expected.Filename)}
 		}
 	}
 	if preflightConflict {
 		result.State = Conflict
-		return result, &forge.Failure{Kind: forge.FailureConflict, Err: errors.New("one or more existing attachments conflict")}
+		return result, &forge.Failure{Kind: forge.FailureConflict, Err: errors.Join(errors.New("one or more existing attachments conflict"), errors.Join(inspectionErrors...))}
 	}
 	if preflightUnverifiable {
 		result.State = NotAttempted
-		return result, &forge.Failure{Kind: forge.FailureUnverifiable, Err: errors.New("one or more existing attachments could not be verified")}
+		return result, &forge.Failure{Kind: forge.FailureUnverifiable, Err: errors.Join(errors.New("one or more existing attachments could not be verified"), errors.Join(inspectionErrors...))}
 	}
 	if len(pending) == 0 {
 		result.State = Complete

internal/delivery/remote_assets_test.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 
 	"go.maximumdirect.net/releasekit/internal/artifact"
@@ -173,6 +174,35 @@ func TestUploadMissingAssetsStopsBeforeWritesForConflictOrUnverifiableContent(t
 	}
 }
 
+func TestUploadMissingAssetsPreservesInspectionCausesAndConflictPrecedence(t *testing.T) {
+	readErr := errors.New("asset download failed")
+	for _, conflict := range []bool{false, true} {
+		name := "unverifiable"
+		if conflict {
+			name = "conflict"
+		}
+		t.Run(name, func(t *testing.T) {
+			backend := &assetBackend{}
+			inspection := Inspection{Assets: []AssetInspection{
+				{Expected: expectedAsset("canceled.bin", []byte("a")), State: AssetUnverifiable, Err: context.Canceled},
+				{Expected: expectedAsset("unreadable.bin", []byte("b")), State: AssetUnverifiable, Err: readErr},
+			}}
+			wantState, wantKind := NotAttempted, forge.FailureUnverifiable
+			if conflict {
+				inspection.Assets = append(inspection.Assets, AssetInspection{Expected: expectedAsset("conflicting.bin", []byte("c")), State: AssetConflicting})
+				wantState, wantKind = Conflict, forge.FailureConflict
+			}
+			result, err := uploadMissingAssets(context.Background(), backend, "release", inspection)
+			if !isForgeFailure(err, wantKind) || result.State != wantState || len(result.Uncertain) != 2 || len(backend.uploadNames) != 0 {
+				t.Fatalf("result = %#v, error %v", result, err)
+			}
+			if !errors.Is(err, context.Canceled) || !errors.Is(err, readErr) || !strings.Contains(err.Error(), "canceled.bin") || !strings.Contains(err.Error(), "unreadable.bin") {
+				t.Fatalf("aggregate lost cause or asset context: %v", err)
+			}
+		})
+	}
+}
+
 func TestUploadMissingAssetsReconcilesOnceAndKeepsLaterAssetsPending(t *testing.T) {
 	assets := []artifact.ExpectedAsset{
 		expectedAsset("a.bin", []byte("a")),