diff --git a/cmd/releasekit/main.go b/cmd/releasekit/main.go index 92fee86..c0948bd 100644 --- a/cmd/releasekit/main.go +++ b/cmd/releasekit/main.go @@ -426,20 +426,7 @@ func deliverWithDestination(ctx context.Context, invocation cli.Invocation, sett Output: stderr, }) if err != nil { - fmt.Fprintf(stderr, "delivery state=%s module=%s module-dir=%s version=%s tag=%s commit=%s url=%s: %v\n", result.State, result.ModulePath, result.ModuleDir, result.Version, result.Tag, result.CandidateCommit, result.ReleaseURL, err) - writeDeliveryAssets(stderr, result) - writeDeliveryRecovery(stderr, result) - if errors.Is(err, releasekit.ErrInvalidVersion) { - return 2 - } - var preflightErr *releasekit.DeliveryPreflightError - if errors.As(err, &preflightErr) { - return deliveryPreflightExitStatus(preflightErr) - } - if result.State == releasekit.DeliveryUnknown { - return 3 - } - return 1 + return reportDeliveryFailure(stderr, result, err) } fmt.Fprintf(stdout, "delivered module %s version %s tag %s at %s (state %s)\n", result.ModulePath, result.Version, result.Tag, result.CandidateCommit, result.State) if result.ReleaseURL != "" { @@ -449,6 +436,29 @@ func deliverWithDestination(ctx context.Context, invocation cli.Invocation, sett return 0 } +func reportDeliveryFailure(output io.Writer, result releasekit.DeliveryResult, err error) int { + if result.State == releasekit.DeliveryComplete { + fmt.Fprintln(output, "hosted delivery completed; local cleanup failed") + } + fmt.Fprintf(output, "delivery state=%s module=%s module-dir=%s version=%s tag=%s commit=%s url=%s: %v\n", result.State, result.ModulePath, result.ModuleDir, result.Version, result.Tag, result.CandidateCommit, result.ReleaseURL, err) + writeDeliveryAssets(output, result) + writeDeliveryRecovery(output, result) + if result.State == releasekit.DeliveryComplete { + return 1 + } + if errors.Is(err, releasekit.ErrInvalidVersion) { + return 2 + } + var preflightErr *releasekit.DeliveryPreflightError + if errors.As(err, &preflightErr) { + return deliveryPreflightExitStatus(preflightErr) + } + if result.State == releasekit.DeliveryUnknown { + return 3 + } + return 1 +} + func deliveryPreflightExitStatus(err error) int { var preflightErr *releasekit.DeliveryPreflightError if !errors.As(err, &preflightErr) { diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index 14c1e2f..93766b9 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -1061,6 +1061,15 @@ func TestDeliverRejectsInvalidTaggedSourceBeforeManagedBuild(t *testing.T) { } } +func TestCompletedDeliveryCleanupFailureReportsCompletedState(t *testing.T) { + var output bytes.Buffer + result := releasekit.DeliveryResult{State: releasekit.DeliveryComplete, ModulePath: "example.com/app", Tag: "v1.2.3", ReleaseURL: "https://example.invalid/release", Uploaded: []releasekit.DeliveryAsset{{Filename: "app", SHA256: "digest"}}} + status := reportDeliveryFailure(&output, result, errors.New("remove temporary directory: failed")) + if status != 1 || !strings.Contains(output.String(), "hosted delivery completed; local cleanup failed") || !strings.Contains(output.String(), "uploaded app digest") || strings.Contains(output.String(), "retry") { + t.Fatalf("status = %d, output = %q", status, output.String()) + } +} + func TestDeliverFailureReportsRecoveryForPartialConflictAndUnknownStates(t *testing.T) { for _, test := range []struct { name string diff --git a/delivery.go b/delivery.go index d363f16..5133927 100644 --- a/delivery.go +++ b/delivery.go @@ -116,7 +116,8 @@ type DeliveryRequest struct { } // DeliveryResult reports hosted content and the state reached by this -// invocation. A partial result may accompany an error. The asset slices are +// invocation. A result may accompany an error, including Complete when hosted +// delivery succeeded but local managed-build cleanup failed. The asset slices are // current observations: an asset appears in at most one slice, and early // failures may leave some declared assets unreported. type DeliveryResult struct { diff --git a/docs/cli.md b/docs/cli.md index ff632a4..79ac0ee 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -209,8 +209,9 @@ first and then run `deliver --assets DIR` with the token available. The ordinary `deliver` command does not download application dependencies or change the checkout. -The command returns 0 for complete delivery, 1 for an unavailable preflight or -a known delivery failure, conflict, or partial delivery, and 2 for usage, +The command returns 0 for complete delivery, 1 for an unavailable preflight, +a known delivery failure, conflict, partial delivery, or local cleanup failure +after hosted completion, and 2 for usage, profile, forge-selection, local credential-loading, or classified preflight configuration/access errors. A remote write whose outcome remains uncertain returns 3, including when cancellation prevents reconciliation. Cancellation diff --git a/docs/consumers/delivery.md b/docs/consumers/delivery.md index b1f4867..28c036d 100644 --- a/docs/consumers/delivery.md +++ b/docs/consumers/delivery.md @@ -154,6 +154,10 @@ an upload was confirmed during this invocation; `Pending` means known missing work; `Conflicting` means different hosted content was found; `Uncertain` means the remote state could not be resolved. +An error can accompany `complete` when hosted delivery succeeded but removal +of managed build files failed. Address the reported local cleanup failure; +the completed hosted delivery does not need to be repeated. + Fix local or preflight failures before retrying. For partial delivery, fix the reported cause and retry the same version. Resolve an unknown remote outcome before retrying. Investigate conflicts and use a new version if the source or diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 5c5610a..71ded96 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -46,7 +46,7 @@ the agent policy; an intermediate compilation dependency is recorded accurately. | 8 | A06, A14 | Complete — public build and prebuilt-delivery checks and CLI receipt/partial-artifact regressions passed. | | 9 | A16 | Complete — shared declaration, receipt, delivery, prebuilt, and source-only regressions passed. | | 10 | A07: allocation and temporary metadata | Complete — allocation resolution, joined cleanup causes, tag evidence, and receipt installation checks passed. | -| 11 | A07: build and delivery outcomes | Pending | +| 11 | A07: build and delivery outcomes | Complete — managed and native cleanup tests cover success, failure, cancellation, hosted completion, and CLI reporting. | | 12 | A08 | Pending | | 13 | A09 | Pending | | 14 | A10 | Pending | diff --git a/internal/build/build.go b/internal/build/build.go index bfc4e77..f51f0c4 100644 --- a/internal/build/build.go +++ b/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" diff --git a/internal/build/candidate.go b/internal/build/candidate.go index b291f11..8d9cbc0 100644 --- a/internal/build/candidate.go +++ b/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, diff --git a/internal/build/cleanup_test.go b/internal/build/cleanup_test.go new file mode 100644 index 0000000..e23f5b7 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/internal/delivery/cleanup_test.go b/internal/delivery/cleanup_test.go new file mode 100644 index 0000000..d8819d3 --- /dev/null +++ b/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) + } + } + }) + } +} diff --git a/internal/delivery/delivery.go b/internal/delivery/delivery.go index 7cf6236..9b124d5 100644 --- a/internal/delivery/delivery.go +++ b/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,