Make pending asset bookkeeping linear

docs/roadmap/remediation.md

@@ -263,7 +263,14 @@ validation behavior across the two supported CI paths.
 
 ## 6. Make pending-asset bookkeeping linear — P3
 
-Status: open.
+Status: complete.
+
+**Completed:** Replaced repeated slice removals with pending-map updates and a
+single deferred compaction pass on every return path. Category bookkeeping is
+now expected O(`n`) with O(`n`) metadata, preserving pending observation order.
+Added an early-return regression with out-of-order observations for definitive
+and uncertain failures. Delivery/Gitea tests and focused facade/CLI recovery
+tests passed.
 
 **Evidence:** `uploadMissingAssets`, `reconcileAssetUpload`, and
 `removeIdentity` in [shared asset delivery](../../internal/delivery/remote_assets.go).

internal/delivery/remote_assets.go

@@ -113,15 +113,26 @@ func remoteAssetDigest(ctx context.Context, backend forge.Backend, handle any) (
 	return hex.EncodeToString(hasher.Sum(nil)), nil
 }
 
-func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHandle any, inspection Inspection) (UploadResult, error) {
+func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHandle any, inspection Inspection) (result UploadResult, err error) {
 	expected := orderedExpected(inspection.Assets)
 	if err := artifact.ValidateExpectedAssets(expected); err != nil {
 		return UploadResult{State: Partial}, &forge.Failure{Kind: forge.FailureInvalid, Err: err}
 	}
-	result := UploadResult{State: Partial}
+	result = UploadResult{State: Partial}
 	preflightConflict := false
 	preflightUnverifiable := false
-	missing := make(map[string]artifact.ExpectedAsset, len(expected))
+	pending := make(map[string]struct{}, len(expected))
+	// Keep the initial observation order and compact pending identities once,
+	// including on early returns after partial or unresolved uploads.
+	defer func() {
+		remaining := result.Pending[:0]
+		for _, item := range result.Pending {
+			if _, ok := pending[item.Filename]; ok {
+				remaining = append(remaining, item)
+			}
+		}
+		result.Pending = remaining
+	}()
 	for _, observation := range inspection.Assets {
 		item := identity(observation.Expected)
 		switch observation.State {
@@ -129,7 +140,7 @@ func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHand
 			result.Matching = append(result.Matching, item)
 		case AssetMissing:
 			result.Pending = append(result.Pending, item)
-			missing[observation.Expected.Filename] = observation.Expected
+			pending[observation.Expected.Filename] = struct{}{}
 		case AssetConflicting:
 			preflightConflict = true
 			result.Conflicting = append(result.Conflicting, item)
@@ -148,13 +159,13 @@ func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHand
 		result.State = NotAttempted
 		return result, &forge.Failure{Kind: forge.FailureUnverifiable, Err: errors.New("one or more existing attachments could not be verified")}
 	}
-	if len(missing) == 0 {
+	if len(pending) == 0 {
 		result.State = Complete
 		return result, nil
 	}
 
 	for _, item := range orderMissing(expected) {
-		if _, needsUpload := missing[item.Filename]; !needsUpload {
+		if _, needsUpload := pending[item.Filename]; !needsUpload {
 			continue
 		}
 		content, err := assetContent(item)
@@ -165,18 +176,19 @@ func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHand
 		closeErr := content.Close()
 		if uploadErr == nil {
 			result.Uploaded = append(result.Uploaded, identity(item))
-			result.Pending = removeIdentity(result.Pending, item.Filename)
+			delete(pending, item.Filename)
 			continue
 		}
 		operationErr := errors.Join(uploadErr, closeErr)
 		if evidence.Disposition != forge.WriteConflict && evidence.Disposition != forge.WriteUncertain {
 			return result, operationErr
 		}
+		delete(pending, item.Filename)
 		if err := reconcileAssetUpload(ctx, backend, releaseHandle, item, evidence, operationErr, &result); err != nil {
 			return result, err
 		}
 	}
-	if len(result.Pending) == 0 {
+	if len(pending) == 0 {
 		result.State = Complete
 	}
 	return result, nil
@@ -186,7 +198,6 @@ func reconcileAssetUpload(ctx context.Context, backend forge.Backend, releaseHan
 	inspection, inspectErr := inspectRemoteAssets(ctx, backend, releaseHandle, []artifact.ExpectedAsset{item})
 	if inspectErr != nil || len(inspection.Assets) != 1 {
 		result.State = Unknown
-		result.Pending = removeIdentity(result.Pending, item.Filename)
 		result.Uncertain = append(result.Uncertain, identity(item))
 		return &forge.Failure{Kind: forge.FailureUnknown, Err: errors.Join(uploadErr, inspectErr)}
 	}
@@ -194,15 +205,12 @@ func reconcileAssetUpload(ctx context.Context, backend forge.Backend, releaseHan
 	switch observation.State {
 	case AssetMatching:
 		result.Matching = append(result.Matching, identity(item))
-		result.Pending = removeIdentity(result.Pending, item.Filename)
 		return nil
 	case AssetConflicting:
 		result.State = Conflict
-		result.Pending = removeIdentity(result.Pending, item.Filename)
 		result.Conflicting = append(result.Conflicting, identity(item))
 		return &forge.Failure{Kind: forge.FailureConflict, Err: uploadErr}
 	case AssetMissing:
-		result.Pending = removeIdentity(result.Pending, item.Filename)
 		if evidence.Disposition == forge.WriteConflict {
 			result.State = Conflict
 			result.Conflicting = append(result.Conflicting, identity(item))
@@ -213,7 +221,6 @@ func reconcileAssetUpload(ctx context.Context, backend forge.Backend, releaseHan
 		return &forge.Failure{Kind: forge.FailureUnknown, Err: uploadErr}
 	default:
 		result.State = Unknown
-		result.Pending = removeIdentity(result.Pending, item.Filename)
 		result.Uncertain = append(result.Uncertain, identity(item))
 		return &forge.Failure{Kind: forge.FailureUnknown, Err: errors.Join(uploadErr, observation.Err)}
 	}
@@ -251,12 +258,3 @@ func orderedExpected(observations []AssetInspection) []artifact.ExpectedAsset {
 func identity(asset artifact.ExpectedAsset) AssetIdentity {
 	return AssetIdentity{Filename: asset.Filename, SHA256: asset.SHA256}
 }
-
-func removeIdentity(identities []AssetIdentity, filename string) []AssetIdentity {
-	for index, item := range identities {
-		if item.Filename == filename {
-			return append(identities[:index], identities[index+1:]...)
-		}
-	}
-	return identities
-}

internal/delivery/remote_assets_test.go

@@ -276,6 +276,38 @@ func TestUploadMissingAssetsPreservesAcknowledgedProgressOnDefinitiveFailure(t *
 	}
 }
 
+func TestUploadMissingAssetsRetainsPendingObservationOrderOnEarlyReturn(t *testing.T) {
+	assets := []artifact.ExpectedAsset{
+		expectedAsset("z.bin", []byte("z")),
+		expectedAsset("a.bin", []byte("a")),
+		expectedAsset("c.bin", []byte("c")),
+		expectedAsset("b.bin", []byte("b")),
+	}
+	for _, disposition := range []forge.WriteDisposition{forge.WriteRejected, forge.WriteUncertain} {
+		t.Run(string(disposition), func(t *testing.T) {
+			backend := &assetBackend{uploads: []assetUploadResult{
+				{evidence: forge.WriteEvidence{Attempted: true, Disposition: forge.WriteAccepted}},
+				{evidence: forge.WriteEvidence{Attempted: true, Disposition: disposition}, err: errors.New("upload stopped")},
+			}}
+			result, err := uploadMissingAssets(context.Background(), backend, "release", inspectionFor(assets, map[string]AssetState{
+				"z.bin": AssetMissing, "a.bin": AssetMissing, "c.bin": AssetMissing, "b.bin": AssetMissing,
+			}))
+			wantPending := []AssetIdentity{identity(assets[0]), identity(assets[2]), identity(assets[3])}
+			wantState := Partial
+			if disposition == forge.WriteUncertain {
+				wantPending = wantPending[:2]
+				wantState = Unknown
+				if !reflect.DeepEqual(result.Uncertain, []AssetIdentity{identity(assets[3])}) {
+					t.Fatalf("uncertain = %#v", result.Uncertain)
+				}
+			}
+			if err == nil || result.State != wantState || !reflect.DeepEqual(result.Pending, wantPending) || !reflect.DeepEqual(result.Uploaded, []AssetIdentity{identity(assets[1])}) {
+				t.Fatalf("result = %#v, error %v; want pending %#v", result, err, wantPending)
+			}
+		})
+	}
+}
+
 func TestUploadMissingAssetsBorrowsReaderAndClosesItAfterCall(t *testing.T) {
 	filePath := t.TempDir() + "/asset.bin"
 	if err := os.WriteFile(filePath, []byte("asset"), 0o600); err != nil {