From b744346210196a8ed0b42490e3889de4d6aeaa47 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 6 Oct 2026 00:54:49 +0000 Subject: [PATCH] Distinguish asset read failures from unknown writes --- cmd/releasekit/main_test.go | 57 +++++++++++++++++++++++++++-------- docs/integrations/gitea.md | 4 ++- docs/roadmap/remediation.md | 8 +++++ internal/delivery/delivery.go | 3 ++ internal/gitea/upload.go | 13 ++++---- 5 files changed, 66 insertions(+), 19 deletions(-) diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index e7253b4..b4ef570 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -423,18 +423,19 @@ func writeDeliveryReceipt(t *testing.T, repository string, settings cli.BuildSet } type deliveryServerState struct { - 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 + malformedCreate bool + failAssetListing bool + failAssetDownloads 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 { @@ -530,6 +531,10 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve _ = json.NewEncoder(w).Encode(map[string]any{"id": state.assetUploads, "name": name, "size": len(content)}) default: if strings.HasPrefix(r.URL.Path, "/files/") { + if state.failAssetDownloads { + http.Error(w, "unavailable", 503) + return + } name := strings.TrimPrefix(r.URL.Path, "/files/") content, ok := state.assets[name] if !ok { @@ -704,3 +709,31 @@ func TestRecoveredPageCreatePreservesPartialDeliveryOnInspectionFailure(t *testi t.Fatalf("status=%d state=%#v %s", status, state, stderr.String()) } } + +func TestUnreadableAssetsDoNotClaimAnUnknownWrite(t *testing.T) { + for _, existingPage := range []bool{false, true} { + t.Run(fmt.Sprintf("existing-page-%t", existingPage), func(t *testing.T) { + repository := newCLIRepository(t) + state := &deliveryServerState{pageExists: existingPage, noteBody: "# Example Product v1.2.3\n\nRelease note.\n", failAssetDownloads: true, assets: map[string][]byte{"helper-v1.2.3-linux-amd64": []byte("fixture artifact")}} + 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) + expectedState := "partial" + expectedCreates := 1 + if existingPage { + expectedState = "not_attempted" + expectedCreates = 0 + } + if status != 1 || !strings.Contains(stderr.String(), "state="+expectedState) || state.assetUploads != 0 || state.releaseCreates != expectedCreates { + t.Fatalf("status=%d creates=%d uploads=%d %s", status, state.releaseCreates, state.assetUploads, stderr.String()) + } + }) + } +} diff --git a/docs/integrations/gitea.md b/docs/integrations/gitea.md index 39fb0aa..7a32c6a 100644 --- a/docs/integrations/gitea.md +++ b/docs/integrations/gitea.md @@ -43,7 +43,9 @@ missing, conflicting, or unverifiable. Same-name attachments with different bytes are conflicts even when their reported size matches. An existing `SHA256SUMS` must exactly match the complete expected manifest; a subset or different manifest is a conflict. An unreadable attachment is unverifiable, -not missing. Unrelated attachments are returned separately and are preserved. +not missing. A failed read-only inspection reports `not_attempted`; if this +invocation already delivered the page, its outcome retains `partial` progress. +Unknown-write outcomes are reserved for unresolved writes. Unrelated attachments are returned separately and are preserved. Attachment downloads and same-origin redirects carry authorization only when their scheme, host, and port match the configured Gitea service. Cross-origin diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 4a10477..be1bad7 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -131,6 +131,8 @@ followed by failed asset inspection. Both affected suites pass. ### R3 — Medium: Read-only asset failures are classified as unknown writes +Status: Resolved. + Category: Divergence from the selected outcome contract. **Evidence:** [UploadMissing](../../internal/gitea/upload.go), around line 102, @@ -153,6 +155,12 @@ as selected in the plan. Adjust tests and documentation consistently. read-only invocation must not return unknown-write state or exit 3. A genuinely ambiguous upload must continue to do so. +**Resolution:** Unverifiable preflight assets now return an inspection failure +with `not_attempted`, retaining partial page progress when creation preceded +the read failure. CLI tests cover existing and newly created pages, no asset +writes, and exit 1. Existing ambiguous-upload regressions still verify unknown +write outcomes. Both affected suites pass. + ### R4 — Medium: Local artifact regeneration is prohibited Category: Inadvertently strengthened prohibition. diff --git a/internal/delivery/delivery.go b/internal/delivery/delivery.go index 0975619..7dc9010 100644 --- a/internal/delivery/delivery.go +++ b/internal/delivery/delivery.go @@ -124,6 +124,9 @@ func Deliver(ctx context.Context, request Request) (Result, error) { result.Conflicting = uploadResult.Conflicting result.Uncertain = uploadResult.Uncertain result.State = State(uploadResult.State) + if result.State == NotAttempted && releaseResult.WriteAttempted { + result.State = Partial + } return result, uploadErr } diff --git a/internal/gitea/upload.go b/internal/gitea/upload.go index 04d88d5..16ea429 100644 --- a/internal/gitea/upload.go +++ b/internal/gitea/upload.go @@ -21,10 +21,11 @@ import ( type DeliveryState string const ( - DeliveryComplete DeliveryState = "complete" - DeliveryPartial DeliveryState = "partial" - DeliveryConflict DeliveryState = "conflict" - DeliveryUnknown DeliveryState = "unknown" + DeliveryNotAttempted DeliveryState = "not_attempted" + DeliveryComplete DeliveryState = "complete" + DeliveryPartial DeliveryState = "partial" + DeliveryConflict DeliveryState = "conflict" + DeliveryUnknown DeliveryState = "unknown" ) type AssetIdentity struct { @@ -100,8 +101,8 @@ func (c Client) UploadMissing(ctx context.Context, destination Destination, rele return result, failure(FailureConflict, "preflight release assets", 0, errors.New("one or more existing attachments conflict")) } if preflightUnknown { - result.State = DeliveryUnknown - return result, failure(FailureUnknown, "preflight release assets", 0, errors.New("one or more existing attachments could not be verified")) + result.State = DeliveryNotAttempted + return result, failure(FailureUnverifiable, "preflight release assets", 0, errors.New("one or more existing attachments could not be verified")) } if len(expected) == 0 { result.State = DeliveryComplete