Preserve delivery outcomes when cleanup fails

internal/build/build.go

@@ -56,9 +56,13 @@ type Result struct {
 }
 
 func BuildResolved(ctx context.Context, request Request) (Result, error) {
+	return buildResolved(ctx, request, os.RemoveAll)
+}
+
+func buildResolved(ctx context.Context, request Request, remove func(string) error) (result Result, resultErr error) {
 	identity := request.Identity
 	settings := artifact.NormalizeSettings(request.Spec.MainPackage, request.Spec.BinaryName, request.Spec.Targets)
-	result := Result{
+	result = Result{
 		RepositoryDir: identity.RepositoryDir,
 		ModuleDir:     identity.ModuleDir,
 		ModulePath:    identity.ModulePath,
@@ -170,7 +174,11 @@ func BuildResolved(ctx context.Context, request Request) (Result, error) {
 		if err != nil {
 			return result, fmt.Errorf("create native smoke directory: %w", err)
 		}
-		defer os.RemoveAll(temporaryDir)
+		defer func() {
+			if err := remove(temporaryDir); err != nil {
+				resultErr = errors.Join(resultErr, fmt.Errorf("remove native smoke directory %q: %w", temporaryDir, err))
+			}
+		}()
 		smokePath = filepath.Join(temporaryDir, settings.BinaryName)
 		if host.OS == "windows" {
 			smokePath += ".exe"

internal/build/candidate.go

@@ -2,6 +2,7 @@ package build
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -14,12 +15,16 @@ import (
 )
 
 func CandidateCallback(spec *Spec, output io.Writer) func(context.Context, candidate.Candidate, *gotool.Owner) error {
+	return candidateCallback(spec, output, os.RemoveAll)
+}
+
+func candidateCallback(spec *Spec, output io.Writer, remove func(string) error) func(context.Context, candidate.Candidate, *gotool.Owner) error {
 	if spec == nil || len(spec.Targets) == 0 {
 		return nil
 	}
 	selected := *spec
 	selected.Targets = append([]Target(nil), spec.Targets...)
-	return func(ctx context.Context, captured candidate.Candidate, owner *gotool.Owner) error {
+	return func(ctx context.Context, captured candidate.Candidate, owner *gotool.Owner) (resultErr error) {
 		moduleSubdir, err := filepath.Rel(captured.RepositoryDir, captured.ModuleDir)
 		if err != nil {
 			return fmt.Errorf("resolve captured module path: %w", err)
@@ -39,7 +44,11 @@ func CandidateCallback(spec *Spec, output io.Writer) func(context.Context, candi
 		if err != nil {
 			return fmt.Errorf("create temporary candidate build output: %w", err)
 		}
-		defer os.RemoveAll(temporaryDir)
+		defer func() {
+			if err := remove(temporaryDir); err != nil {
+				resultErr = errors.Join(resultErr, fmt.Errorf("remove candidate build directory %q: %w", temporaryDir, err))
+			}
+		}()
 		_, err = BuildResolved(ctx, Request{
 			Identity:  identity,
 			Spec:      selected,

internal/build/cleanup_test.go

@@ -0,0 +1,124 @@
+package build
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"go.maximumdirect.net/releasekit/internal/candidate"
+	"go.maximumdirect.net/releasekit/internal/releaseidentity"
+)
+
+func cleanupFixture(t *testing.T, output string) releaseidentity.Identity {
+	t.Helper()
+	repo := t.TempDir()
+	files := map[string]string{
+		"go.mod":  "module example.com/cleanup\n\ngo 1.25.0\n",
+		"main.go": fmt.Sprintf("package main\nimport \"fmt\"\nfunc main() { fmt.Println(%q) }\n", output),
+	}
+	for name, contents := range files {
+		if err := os.WriteFile(filepath.Join(repo, name), []byte(contents), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, args := range [][]string{{"init", "-b", "main"}, {"config", "user.name", "Fixture"}, {"config", "user.email", "fixture@example.invalid"}, {"add", "."}, {"commit", "-m", "fixture"}} {
+		command := exec.Command("git", args...)
+		command.Dir = repo
+		if data, err := command.CombinedOutput(); err != nil {
+			t.Fatalf("git %v: %v: %s", args, err, data)
+		}
+	}
+	return releaseidentity.Identity{Selection: releaseidentity.Selection{RepositoryDir: repo, ModuleDir: repo}, ModulePath: "example.com/cleanup", Version: "v1.2.3", Tag: "v1.2.3"}
+}
+
+type cancelWriter struct{ cancel context.CancelFunc }
+
+func (writer cancelWriter) Write(p []byte) (int, error) {
+	if strings.Contains(string(p), "checking ") {
+		writer.cancel()
+	}
+	return len(p), nil
+}
+
+func TestNativeSmokeCleanupPreservesBuildOutcome(t *testing.T) {
+	for _, mode := range []string{"success", "smoke error", "cancellation"} {
+		t.Run(mode, func(t *testing.T) {
+			output := "fixture v1.2.3"
+			if mode == "smoke error" {
+				output = "wrong"
+			}
+			identity := cleanupFixture(t, output)
+			otherArch := "arm64"
+			if runtime.GOARCH == otherArch {
+				otherArch = "amd64"
+			}
+			ctx, cancel := context.WithCancel(t.Context())
+			defer cancel()
+			request := Request{Identity: identity, OutputDir: t.TempDir(), Spec: Spec{MainPackage: ".", BinaryName: "fixture", Targets: []Target{{OS: runtime.GOOS, Arch: otherArch}}}}
+			if mode == "cancellation" {
+				request.Output = cancelWriter{cancel}
+			}
+			cleanupErr := errors.New("cleanup failed")
+			var removed string
+			result, err := buildResolved(ctx, request, func(path string) error {
+				removed = path
+				_ = os.RemoveAll(path)
+				return cleanupErr
+			})
+			if !errors.Is(err, cleanupErr) || removed == "" || !strings.Contains(err.Error(), removed) || len(result.Artifacts) != 1 || result.Artifacts[0].SHA256 == "" || result.CandidateCommit == "" {
+				t.Fatalf("build = %#v, cleanup = %q, error = %v", result, removed, err)
+			}
+			if mode == "success" && !result.SourceVerified {
+				t.Fatal("completed build lost verified source")
+			}
+			if mode == "smoke error" && !strings.Contains(err.Error(), "native version output") {
+				t.Fatalf("smoke failure missing: %v", err)
+			}
+			if mode == "cancellation" && !errors.Is(err, context.Canceled) {
+				t.Fatalf("cancellation missing: %v", err)
+			}
+		})
+	}
+}
+
+func TestCandidateBuildCleanupPreservesOutcome(t *testing.T) {
+	for _, mode := range []string{"success", "build failure", "cancellation"} {
+		t.Run(mode, func(t *testing.T) {
+			identity := cleanupFixture(t, "fixture v1.2.3")
+			cleanupErr := errors.New("cleanup failed")
+			var removed string
+			spec := &Spec{MainPackage: ".", BinaryName: "fixture", Targets: []Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}}
+			if mode == "build failure" {
+				spec.MainPackage = "./missing"
+			}
+			ctx, cancel := context.WithCancel(t.Context())
+			defer cancel()
+			var output io.Writer
+			if mode == "cancellation" {
+				output = cancelWriter{cancel}
+			}
+			callback := candidateCallback(spec, output, func(path string) error {
+				removed = path
+				_ = os.RemoveAll(path)
+				return cleanupErr
+			})
+			err := callback(ctx, candidate.Candidate{RepositoryDir: identity.RepositoryDir, ModuleDir: identity.ModuleDir, ModulePath: identity.ModulePath, Version: identity.Version, Tag: identity.Tag}, nil)
+			if !errors.Is(err, cleanupErr) || removed == "" || !strings.Contains(err.Error(), removed) {
+				t.Fatalf("candidate cleanup = %q, error = %v", removed, err)
+			}
+			if mode == "build failure" && !strings.Contains(err.Error(), "build candidate") {
+				t.Fatalf("build failure missing: %v", err)
+			}
+			if mode == "cancellation" && !errors.Is(err, context.Canceled) {
+				t.Fatalf("cancellation missing: %v", err)
+			}
+		})
+	}
+}

internal/delivery/cleanup_test.go

@@ -0,0 +1,102 @@
+package delivery
+
+import (
+	"context"
+	"errors"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"go.maximumdirect.net/releasekit/internal/artifact"
+	internalbuild "go.maximumdirect.net/releasekit/internal/build"
+	"go.maximumdirect.net/releasekit/internal/forge"
+)
+
+func deliveryCleanupFixture(t *testing.T) (string, string) {
+	t.Helper()
+	repo := t.TempDir()
+	files := map[string]string{
+		"go.mod":                  "module example.com/delivery\n\ngo 1.25.0\n",
+		"main.go":                 "package main\nimport \"fmt\"\nfunc main() { fmt.Println(\"fixture v1.2.3\") }\n",
+		"docs/releases/v1.2.3.md": "# Fixture v1.2.3\n\nNote.\n",
+	}
+	for name, contents := range files {
+		path := filepath.Join(repo, filepath.FromSlash(name))
+		if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(path, []byte(contents), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, args := range [][]string{{"init", "-b", "main"}, {"config", "user.name", "Fixture"}, {"config", "user.email", "fixture@example.invalid"}, {"add", "."}, {"commit", "-m", "fixture"}, {"tag", "v1.2.3"}} {
+		command := exec.Command("git", args...)
+		command.Dir = repo
+		if data, err := command.CombinedOutput(); err != nil {
+			t.Fatalf("git %v: %v: %s", args, err, data)
+		}
+	}
+	command := exec.Command("git", "rev-parse", "HEAD")
+	command.Dir = repo
+	commit, err := command.Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+	return repo, strings.TrimSpace(string(commit))
+}
+
+type cancelBuildOutput struct{ cancel context.CancelFunc }
+
+func (writer cancelBuildOutput) Write(p []byte) (int, error) {
+	if strings.Contains(string(p), "building ") {
+		writer.cancel()
+	}
+	return len(p), nil
+}
+
+func TestManagedDeliveryCleanupPreservesOutcome(t *testing.T) {
+	for _, mode := range []string{"complete", "build failure", "cancellation"} {
+		t.Run(mode, func(t *testing.T) {
+			repo, commit := deliveryCleanupFixture(t)
+			note := "# Fixture v1.2.3\n\nNote.\n"
+			backend := &assetBackend{pageBackend: pageBackend{commit: commit, findResults: []pageFindResult{{release: forge.Release{Tag: "v1.2.3", Body: note, Handle: "release", URL: "https://example.invalid/release"}}}}, lists: []assetListResult{{}}}
+			ctx, cancel := context.WithCancel(t.Context())
+			defer cancel()
+			spec := internalbuild.Spec{MainPackage: ".", BinaryName: "fixture", Targets: []artifact.Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}}
+			if mode == "build failure" {
+				spec.MainPackage = "./missing"
+			}
+			request := Request{RepositoryDir: repo, Product: "Fixture", Version: "v1.2.3", Backend: backend, Build: &spec}
+			if mode == "cancellation" {
+				request.Output = cancelBuildOutput{cancel}
+			}
+			cleanupErr := errors.New("cleanup failed")
+			var removed string
+			result, err := deliver(ctx, request, func(path string) error {
+				removed = path
+				_ = os.RemoveAll(path)
+				return cleanupErr
+			})
+			if !errors.Is(err, cleanupErr) || removed == "" || !strings.Contains(err.Error(), removed) || result.CandidateCommit != commit {
+				t.Fatalf("delivery = %#v, cleanup = %q, error = %v", result, removed, err)
+			}
+			switch mode {
+			case "complete":
+				if result.State != Complete || len(result.Uploaded) != 2 || result.ReleaseURL == "" {
+					t.Fatalf("completed delivery lost observations: %#v", result)
+				}
+			case "build failure":
+				if result.State != NotAttempted || !strings.Contains(err.Error(), "build delivery assets") {
+					t.Fatalf("build failure lost: %#v, %v", result, err)
+				}
+			case "cancellation":
+				if result.State != NotAttempted || !errors.Is(err, context.Canceled) {
+					t.Fatalf("cancellation lost: %#v, %v", result, err)
+				}
+			}
+		})
+	}
+}

internal/delivery/delivery.go

@@ -58,7 +58,11 @@ type Result struct {
 }
 
 func Deliver(ctx context.Context, request Request) (Result, error) {
-	result := Result{State: NotAttempted}
+	return deliver(ctx, request, os.RemoveAll)
+}
+
+func deliver(ctx context.Context, request Request, remove func(string) error) (result Result, resultErr error) {
+	result = Result{State: NotAttempted}
 	if !versioncontract.ValidStableVersion(request.Version) {
 		return result, candidate.ErrInvalidVersion
 	}
@@ -130,7 +134,11 @@ func Deliver(ctx context.Context, request Request) (Result, error) {
 		if err != nil {
 			return result, fmt.Errorf("create temporary delivery build directory: %w", err)
 		}
-		defer os.RemoveAll(temporaryDir)
+		defer func() {
+			if err := remove(temporaryDir); err != nil {
+				resultErr = errors.Join(resultErr, fmt.Errorf("remove delivery build directory %q: %w", temporaryDir, err))
+			}
+		}()
 		built, err := internalbuild.BuildResolved(ctx, internalbuild.Request{
 			Identity:  identity,
 			Spec:      *request.Build,