From 91712fd900e8419394aea51820f0ea81e1a680c6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 6 Oct 2026 00:53:33 +0000 Subject: [PATCH] Preserve page creation outcomes through recovery --- cmd/releasekit/main_test.go | 49 +++++++++++++++++++++++------- docs/integrations/gitea.md | 6 ++-- docs/roadmap/remediation.md | 8 +++++ internal/delivery/delivery.go | 2 +- internal/gitea/release.go | 13 ++++++-- internal/gitea/release_test.go | 54 ++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+), 16 deletions(-) diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index a4221fe..e7253b4 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -423,16 +423,18 @@ func writeDeliveryReceipt(t *testing.T, repository string, settings cli.BuildSet } type deliveryServerState struct { - commit string - product string - remoteCommit string - apiRequests int - pageExists bool - releaseCreates int - assetUploads int - assetListReads int - noteBody string - assets map[string][]byte + malformedCreate bool + failAssetListing bool + commit string + product string + remoteCommit string + apiRequests int + pageExists bool + releaseCreates int + assetUploads int + assetListReads int + noteBody string + assets map[string][]byte } func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Server { @@ -483,10 +485,19 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve t.Errorf("release source payload = %#v", payload) } state.pageExists, state.releaseCreates, state.noteBody = true, state.releaseCreates+1, payload.Body + if state.malformedCreate { + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "tag_name": "v1.2.3", "name": "inconsistent response", "body": payload.Body}) + return + } writeDeliveryRelease(w, serverURL, state) case "/api/v1/repos/owner/example/releases/10/assets": if r.Method == http.MethodGet { state.assetListReads++ + if state.failAssetListing { + http.Error(w, "unavailable", 503) + return + } list := make([]map[string]any, 0, len(state.assets)) for name, content := range state.assets { list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": serverURL + "/files/" + name}) @@ -675,3 +686,21 @@ func TestDeliverRejectsReceiptBuiltFromDirtySourceAfterCleanup(t *testing.T) { t.Fatalf("status=%d requests=%d %s", status, state.apiRequests, stderr.String()) } } + +func TestRecoveredPageCreatePreservesPartialDeliveryOnInspectionFailure(t *testing.T) { + repository := newCLIRepository(t) + state := &deliveryServerState{malformedCreate: true, failAssetListing: true} + server := newDeliveryServer(t, state) + defer server.Close() + build := cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}} + writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &build, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) + state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + cliGitRun(t, repository, "tag", "v1.2.3") + assets, _ := writeDeliveryReceipt(t, repository, build) + t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") + var stdout, stderr bytes.Buffer + status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assets, "v1.2.3"}, &stdout, &stderr) + if status != 1 || !strings.Contains(stderr.String(), "state=partial") || state.releaseCreates != 1 || state.assetListReads != 1 || state.assetUploads != 0 { + t.Fatalf("status=%d state=%#v %s", status, state, stderr.String()) + } +} diff --git a/docs/integrations/gitea.md b/docs/integrations/gitea.md index 3df78cb..39fb0aa 100644 --- a/docs/integrations/gitea.md +++ b/docs/integrations/gitea.md @@ -24,11 +24,13 @@ conflicting page. If the page is absent, the client posts to `/api/v1/repos/{owner}/{repo}/releases` with the selected tag and commit, the release note's H1 as title, the complete note as body, and draft and prerelease disabled. A create conflict, interrupted -response, or malformed create response triggers one lookup. An exact page found +response, or malformed create response (including inconsistent decoded fields) +triggers one lookup. An exact page found there confirms the existing content; a differing page is a conflict. If that lookup cannot determine the result after a possibly successful write, the client returns an unknown outcome. Other HTTP failures retain their structured -failure kind and status code. +failure kind and status code. A recovered create retains its write history; +a later asset-inspection failure reports partial page progress. For a binary-producing release, the client can build a deterministic `SHA256SUMS` file from the declared artifact names and SHA-256 digests. Entries diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index d587cd9..4a10477 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -96,6 +96,8 @@ cleanup. The offline suite and public boundary checks pass. ### R2 — High: Some successful page writes are reported as not attempted +Status: Resolved. + Category: Failure handling and recovery-state correctness. **Evidence:** [EnsureRelease](../../internal/gitea/release.go), at the @@ -121,6 +123,12 @@ unknown. Retain the selected bounded reconciliation and immutable-content rules. its matching and conflicting follow-up lookups, and a recovered create followed by asset-inspection failure. Assert both request counts and returned state. +**Resolution:** Semantic create-response failures now use one reconciliation +lookup. Recovered page results retain create-request history, preserving partial +progress on later asset-inspection failure. Local HTTP tests cover matching, +conflicting, and unavailable lookups; a CLI fixture verifies recovered creation +followed by failed asset inspection. Both affected suites pass. + ### R3 — Medium: Read-only asset failures are classified as unknown writes Category: Divergence from the selected outcome contract. diff --git a/internal/delivery/delivery.go b/internal/delivery/delivery.go index 90e5371..0975619 100644 --- a/internal/delivery/delivery.go +++ b/internal/delivery/delivery.go @@ -111,7 +111,7 @@ func Deliver(ctx context.Context, request Request) (Result, error) { inspection, err := (gitea.Client{}).InspectAssets(ctx, request.Destination, releaseResult.Release.ID, request.Assets) if err != nil { result.State = NotAttempted - if releaseResult.Created { + if releaseResult.WriteAttempted { result.State = Partial } result.Uncertain = assetIdentities(request.Assets) diff --git a/internal/gitea/release.go b/internal/gitea/release.go index a8e180b..9ce0376 100644 --- a/internal/gitea/release.go +++ b/internal/gitea/release.go @@ -75,8 +75,9 @@ type Release struct { } type Result struct { - Release Release - Created bool + Release Release + Created bool + WriteAttempted bool // a create request was issued, including reconciled creates. } type Client struct { @@ -150,7 +151,12 @@ func (c Client) EnsureRelease(ctx context.Context, request Request) (Result, err } created, postStatus, postErr := c.postRelease(ctx, client, request.Destination.Token, apiRoot+"/releases", body) if postErr == nil { - return acceptCreated(created, request) + result, verifyErr := acceptCreated(created, request) + if verifyErr == nil { + result.WriteAttempted = true + return result, nil + } + postErr = verifyErr } if !shouldReconcileWrite(postStatus, postErr) { return Result{}, postErr @@ -162,6 +168,7 @@ func (c Client) EnsureRelease(ctx context.Context, request Request) (Result, err if retryErr == nil { result, matchErr := acceptExisting(retry, request) if matchErr == nil { + result.WriteAttempted = true return result, nil } return Result{}, failure(FailureConflict, "resolve release creation", postStatus, matchErr) diff --git a/internal/gitea/release_test.go b/internal/gitea/release_test.go index 18fb130..ef7ab8b 100644 --- a/internal/gitea/release_test.go +++ b/internal/gitea/release_test.go @@ -339,3 +339,57 @@ func failureKind(t *testing.T, err error) FailureKind { func contains(value, substring string) bool { return len(substring) > 0 && len(value) >= len(substring) && strings.Contains(value, substring) } + +func TestEnsureReleaseReconcilesInconsistentSuccessfulCreate(t *testing.T) { + for _, mode := range []string{"matching", "conflicting", "unavailable"} { + t.Run(mode, func(t *testing.T) { + posts, lookups := 0, 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/tags/") && !strings.Contains(r.URL.Path, "/releases/"): + writeTag(t, w, testCommit) + case strings.Contains(r.URL.Path, "/releases/tags/"): + lookups++ + if posts == 0 { + http.NotFound(w, r) + return + } + if mode == "unavailable" { + http.Error(w, "unavailable", 503) + return + } + body := testNote + if mode == "conflicting" { + body = "other content" + } + writeRelease(t, w, releaseFixture{title: "Example v1.2.3", body: body}) + case r.Method == http.MethodPost: + posts++ + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(releaseResponse{ID: 10, TagName: "v1.2.3", Name: "inconsistent title", Body: testNote}) + default: + t.Errorf("unexpected request %s", r.URL.Path) + } + })) + defer server.Close() + result, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL)) + if posts != 1 || lookups != 2 { + t.Fatalf("posts=%d lookups=%d", posts, lookups) + } + switch mode { + case "matching": + if err != nil || !result.WriteAttempted { + t.Fatalf("result=%#v err=%v", result, err) + } + case "conflicting": + if failureKind(t, err) != FailureConflict { + t.Fatal(err) + } + case "unavailable": + if failureKind(t, err) != FailureUnknown { + t.Fatal(err) + } + } + }) + } +}