Make delivery asset outcomes disjoint

This commit is contained in:
2026-10-06 17:54:22 +00:00
parent 0eec4a30ed
commit c9afc8dd12
10 changed files with 322 additions and 32 deletions

View File

@@ -230,6 +230,7 @@ func runDelivery(ctx context.Context, invocation cli.Invocation, stdout, stderr
if err != nil {
fmt.Fprintf(stderr, "delivery state=%s version=%s commit=%s url=%s: %v\n", result.State, result.Version, result.CandidateCommit, result.ReleaseURL, err)
writeDeliveryAssets(stderr, result)
writeDeliveryRecovery(stderr, result)
if errors.Is(err, releasekit.ErrInvalidVersion) {
return 2
}
@@ -257,6 +258,24 @@ func writeDeliveryAssets(output io.Writer, result releasekit.DeliveryResult) {
}
}
func writeDeliveryRecovery(output io.Writer, result releasekit.DeliveryResult) {
switch result.State {
case releasekit.DeliveryNotAttempted:
fmt.Fprintln(output, "recovery: fix the local or preflight failure before retrying; no remote write was attempted")
case releasekit.DeliveryPartial:
fmt.Fprintln(output, "recovery: fix the reported cause, then retry this version")
case releasekit.DeliveryConflict:
fmt.Fprintln(output, "recovery: investigate conflicting hosted content; use a new version if the source or published contents need correction")
case releasekit.DeliveryUnknown:
fmt.Fprintln(output, "recovery: resolve the remote state before retrying")
default:
return
}
if len(result.Matching)+len(result.Uploaded)+len(result.Conflicting)+len(result.Uncertain) != 0 {
fmt.Fprintln(output, "if rebuilding could change asset bytes already on the server, retain the original build artifacts and retry with deliver --assets DIR")
}
}
func buildResultFromReceipt(receipt cli.BuildReceipt) *releasekit.BuildResult {
result := &releasekit.BuildResult{
RepositoryDir: receipt.RepositoryDir, CandidateCommit: receipt.CandidateCommit, SourceVerified: receipt.SourceVerified,

View File

@@ -317,6 +317,82 @@ func TestDeliverBuildsTaggedArtifactsAndReusesAssets(t *testing.T) {
}
}
func TestDeliverFailureReportsRecoveryForPartialConflictAndUnknownStates(t *testing.T) {
for _, test := range []struct {
name string
state string
status int
recovery []string
assetObservation string
missingObservation string
}{
{
name: "partial",
status: 1,
recovery: []string{"recovery: fix the reported cause", "retry this version"},
assetObservation: "pending helper-v1.2.3-linux-amd64",
},
{
name: "conflict",
status: 1,
recovery: []string{"investigate conflicting hosted content", "new version", "retain the original build artifacts"},
assetObservation: "conflicting helper-v1.2.3-linux-amd64",
missingObservation: "pending SHA256SUMS",
},
{
name: "unknown",
status: 3,
recovery: []string{"resolve the remote state before retrying"},
assetObservation: "uncertain helper-v1.2.3-linux-amd64",
missingObservation: "pending SHA256SUMS",
},
} {
t.Run(test.name, func(t *testing.T) {
repository := newCLIRepository(t)
state := &deliveryServerState{}
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")
assetsDir, _ := writeDeliveryReceipt(t, repository, build)
switch test.name {
case "partial":
state.failAssetName = "helper-v1.2.3-linux-amd64"
case "conflict":
state.assets["helper-v1.2.3-linux-amd64"] = []byte("different hosted bytes")
case "unknown":
state.ambiguousAssetName = "helper-v1.2.3-linux-amd64"
}
t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token")
var stdout, stderr bytes.Buffer
status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assetsDir, "v1.2.3"}, &stdout, &stderr)
if status != test.status || !strings.Contains(stderr.String(), "state="+test.name) {
t.Fatalf("status=%d output=%q want status=%d state=%s", status, stderr.String(), test.status, test.name)
}
for _, phrase := range test.recovery {
if !strings.Contains(stderr.String(), phrase) {
t.Errorf("delivery output does not include recovery guidance %q: %s", phrase, stderr.String())
}
}
if !strings.Contains(stderr.String(), test.assetObservation) {
t.Errorf("delivery output does not include asset observation %q: %s", test.assetObservation, stderr.String())
}
if test.missingObservation != "" && !strings.Contains(stderr.String(), test.missingObservation) {
t.Errorf("delivery output does not include later missing asset %q: %s", test.missingObservation, stderr.String())
}
if test.name == "unknown" && strings.Contains(stderr.String(), "pending helper-v1.2.3-linux-amd64") {
t.Errorf("uncertain asset also appears pending: %s", stderr.String())
}
})
}
}
func TestDeliverRejectsWrongHostedTagBeforeReleaseWrite(t *testing.T) {
repository := newCLIRepository(t)
state := &deliveryServerState{remoteCommit: strings.Repeat("0", 40)}
@@ -443,6 +519,8 @@ type deliveryServerState struct {
malformedCreate bool
failAssetListing bool
failAssetDownloads bool
failAssetName string
ambiguousAssetName string
commit string
product string
remoteCommit string
@@ -543,6 +621,14 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve
if _, exists := state.assets[name]; exists {
t.Errorf("asset %q uploaded more than once", name)
}
if name == state.failAssetName {
http.Error(w, "upload failed", http.StatusBadRequest)
return
}
if name == state.ambiguousAssetName {
http.Error(w, "upload outcome ambiguous", http.StatusInternalServerError)
return
}
state.assets[name] = content
state.assetUploads++
_ = json.NewEncoder(w).Encode(map[string]any{"id": state.assetUploads, "name": name, "size": len(content)})
@@ -751,6 +837,9 @@ func TestUnreadableAssetsDoNotClaimAnUnknownWrite(t *testing.T) {
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())
}
if existingPage && !strings.Contains(stderr.String(), "recovery: fix the local or preflight failure") {
t.Fatalf("not-attempted recovery guidance missing: %s", stderr.String())
}
})
}
}

View File

@@ -33,6 +33,7 @@ func TestReleaseWrapperPreservesNativeExitStatuses(t *testing.T) {
for _, path := range []string{
"check.go", "publish.go", "internal/candidate/check.go", "internal/publish/publish.go",
"build_test.go", "publish_test.go", "internal/candidate/check_test.go",
"internal/gitea/upload.go", "internal/gitea/upload_test.go",
"cmd/releasekit/main_test.go", "cmd/releasekit/release_wrapper_test.go",
"cmd/releasekit/internal/cli/config_test.go", "examples/releasekit.json",
} {

View File

@@ -49,17 +49,24 @@ type DeliveryRequest struct {
}
// DeliveryResult reports hosted content and the state reached by this
// invocation. A partial result may accompany an error.
// invocation. A partial result may accompany an error. 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 {
Version string
CandidateCommit string
ReleaseURL string
State DeliveryState
Matching []DeliveryAsset
Uploaded []DeliveryAsset
Pending []DeliveryAsset
Conflicting []DeliveryAsset
Uncertain []DeliveryAsset
// Matching lists existing attachments with the declared content.
Matching []DeliveryAsset
// Uploaded lists assets whose uploads were confirmed during this invocation.
Uploaded []DeliveryAsset
// Pending lists assets known to be missing.
Pending []DeliveryAsset
// Conflicting lists assets with detected content conflicts.
Conflicting []DeliveryAsset
// Uncertain lists assets whose remote state could not be resolved.
Uncertain []DeliveryAsset
}
// Deliver verifies the selected local tag and hosted source, then creates or

View File

@@ -105,7 +105,14 @@ build, or manifest. `deliver` never runs candidate checks or publishes a tag.
The command returns 0 for complete delivery, 1 for a known failure, conflict,
or partial delivery, 2 for usage, profile, or missing-credential errors, and 3
when a remote write remains uncertain. The result lists matching, uploaded,
pending, conflicting, and uncertain assets as available. Retry the same version
with the same tagged source and destination; matching content is kept and only
missing assets are added. See the [delivery consumer guide](consumers/delivery.md)
and [Gitea integration guide](integrations/gitea.md).
pending, conflicting, and uncertain assets as disjoint current observations;
early failures may leave some declared assets unreported. On failure, the CLI
prints state-based recovery guidance. Fix local or preflight failures before
retrying; fix the reported cause before retrying partial delivery; resolve an
unknown remote state before retrying; and investigate conflicts, using a new
version if the source or published contents need correction. If rebuilding
could change asset bytes already on the server, retain the original build
artifacts and retry with `--assets DIR`. Matching content is kept and only
known missing assets are added. See the
[delivery consumer guide](consumers/delivery.md) and
[Gitea integration guide](integrations/gitea.md).

View File

@@ -46,9 +46,19 @@ settings, and file hashes before making a remote request.
The result reports `complete`, `partial`, `conflict`, `unknown`, or
`not_attempted`, along with matching, uploaded, pending, conflicting, and
uncertain assets when known. Exit status 3 means a remote write could not be
resolved; inspect the same version and retry only after the remote state is
understood. The command does not run candidate checks or publish tags.
uncertain assets when known. These are disjoint current observations: an asset
appears in at most one category, and early failures may leave some declared
assets unreported. `Matching` means existing content matches; `Uploaded` means
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.
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
published content needs correction. If rebuilding could change bytes already
on the server, retain the original build output and retry with `--assets DIR`.
The command does not run candidate checks or publish tags.
## Go API

View File

@@ -55,11 +55,14 @@ The upload operation accepts only an inspection result without conflicts or
unverifiable assets. It uploads missing binaries in filename order and then the
missing `SHA256SUMS`, using multipart field `attachment` and the filename query
parameter `name`. Matching attachments are retained; unrelated and existing
attachments are never overwritten or deleted. Results keep matching, uploaded,
pending, conflicting, and uncertain identities separate. After an ambiguous
attachments are never overwritten or deleted. Results use disjoint current
observations: matching, uploaded, pending, conflicting, and uncertain. An asset
moved from known missing work to another category is removed from `pending`;
later known-missing assets can remain pending when an earlier upload stops the
operation. Early failures may leave declarations unreported. After an ambiguous
upload or an upload name conflict, the client performs one list and content
check. It does not repeat the POST when the result is missing or cannot be
verified, so a caller can retry with a new inspection.
verified, so a caller can resolve the remote state before retrying.
Uploads use the configured service origin and the same explicit token header.
Source-only page delivery supplies no binary assets and creates no manifest.

View File

@@ -1,6 +1,6 @@
# ReleaseKit audit implementation plan
Status: Stages 1–4 complete; Stage 5 not started.
Status: Stages 1–5 complete; Stage 6 next.
Baseline: `e4be7fa` (2026-10-06). The [audit](audit.md) records the evidence,
product goals, and selected direction. This document owns implementation order
and completion criteria for that work.
@@ -277,6 +277,14 @@ pending assets. Assert category exclusivity and meaningful overall state at the
upload boundary. CLI tests verify recovery meaning and exit classification
without freezing full diagnostic wording.
**Completed:** Delivery asset categories now contain disjoint current
observations, with later known-missing assets retained when an earlier upload
stops work. CLI failures include state-based recovery guidance, while GoDoc and
the delivery and Gitea guides describe category meaning and incomplete early
observations. Upload and CLI tests cover reconciliation outcomes, recovery
classification, and category exclusivity. The full offline suite, API boundary
check, and documentation whitespace checks pass.
## Stage 6 — Give the facade ownership of build-and-deliver
**Covers:** F5. **Primary owners:** `delivery.go`, `internal/delivery/`,

View File

@@ -33,13 +33,21 @@ type AssetIdentity struct {
SHA256 string
}
// UploadResult reports the current asset observations from one upload attempt.
// Each identity appears in at most one category; early failures can leave some
// declared assets unreported.
type UploadResult struct {
State DeliveryState
Matching []AssetIdentity
Uploaded []AssetIdentity
Pending []AssetIdentity
State DeliveryState
// Matching contains existing attachments with the declared content.
Matching []AssetIdentity
// Uploaded contains assets whose upload was confirmed during this invocation.
Uploaded []AssetIdentity
// Pending contains assets known to be missing.
Pending []AssetIdentity
// Conflicting contains assets with detected content conflicts.
Conflicting []AssetIdentity
Uncertain []AssetIdentity
// Uncertain contains assets whose remote state could not be resolved.
Uncertain []AssetIdentity
}
// VerifyLocalAssets checks that every expected file still matches its declared
@@ -146,6 +154,7 @@ func (c Client) UploadMissing(ctx context.Context, destination Destination, rele
inspection, inspectErr := c.InspectAssets(ctx, destination, releaseID, []ExpectedAsset{item})
if inspectErr != nil || len(inspection.Assets) != 1 {
result.State = DeliveryUnknown
result.Pending = removeIdentity(result.Pending, item.Filename)
result.Uncertain = append(result.Uncertain, identity(item))
return result, failure(FailureUnknown, "resolve asset upload", status, errors.Join(uploadErr, inspectErr))
}
@@ -156,15 +165,18 @@ func (c Client) UploadMissing(ctx context.Context, destination Destination, rele
result.Pending = removeIdentity(result.Pending, item.Filename)
case AssetConflicting:
result.State = DeliveryConflict
result.Pending = removeIdentity(result.Pending, item.Filename)
result.Conflicting = append(result.Conflicting, identity(item))
return result, failure(FailureConflict, "resolve asset upload", status, uploadErr)
case AssetMissing:
if status == http.StatusConflict {
result.State = DeliveryConflict
result.Pending = removeIdentity(result.Pending, item.Filename)
result.Conflicting = append(result.Conflicting, identity(item))
return result, failure(FailureConflict, "resolve asset upload", status, uploadErr)
}
result.State = DeliveryUnknown
result.Pending = removeIdentity(result.Pending, item.Filename)
result.Uncertain = append(result.Uncertain, identity(item))
return result, failure(FailureUnknown, "resolve asset upload", status, uploadErr)
default:

View File

@@ -42,6 +42,7 @@ func TestUploadMissingPartialFailureThenRerunOnlyMissingAssets(t *testing.T) {
if got, want := names(first.Pending), []string{"zeta.bin", "beta.bin", "SHA256SUMS"}; !reflect.DeepEqual(got, want) {
t.Fatalf("pending = %v, want %v", got, want)
}
assertAssetCategoriesDisjoint(t, first)
secondInspection, err := client.InspectAssets(context.Background(), destination, 10, assets)
if err != nil {
@@ -80,6 +81,7 @@ func TestUploadMissingAcceptsConcurrentMatchingAttachmentWithoutReplacement(t *t
if len(result.Uploaded) != 0 || len(fixture.postNames()) != 1 {
t.Fatalf("result = %#v, POST names = %v", result, fixture.postNames())
}
assertAssetCategoriesDisjoint(t, result)
}
func TestUploadMissingReconcilesLostManifestAcknowledgment(t *testing.T) {
@@ -111,6 +113,7 @@ func TestUploadMissingReconcilesLostManifestAcknowledgment(t *testing.T) {
if len(result.Uploaded) != 0 || !reflect.DeepEqual(fixture.postNames(), []string{ManifestFilename}) {
t.Fatalf("result = %#v, POST names = %v", result, fixture.postNames())
}
assertAssetCategoriesDisjoint(t, result)
}
func TestUploadMissingReportsUnknownWhenReconciliationReadFails(t *testing.T) {
@@ -119,7 +122,7 @@ func TestUploadMissingReportsUnknownWhenReconciliationReadFails(t *testing.T) {
defer server.Close()
client := testClient(server)
destination := testDestination(server.URL)
assets := localAssets(t, []string{"app.bin"})
assets := localAssets(t, []string{"app.bin", "later.bin"})
initial, err := client.InspectAssets(context.Background(), destination, 10, assets)
if err != nil {
t.Fatal(err)
@@ -128,12 +131,13 @@ func TestUploadMissingReportsUnknownWhenReconciliationReadFails(t *testing.T) {
if err == nil || result.State != DeliveryUnknown {
t.Fatalf("result = %#v, error = %v", result, err)
}
if got, want := names(result.Pending), []string{"app.bin"}; !reflect.DeepEqual(got, want) {
if got, want := names(result.Pending), []string{"later.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("pending = %v, want %v", got, want)
}
if got, want := names(result.Uncertain), []string{"app.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("uncertain = %v, want %v", got, want)
}
assertAssetCategoriesDisjoint(t, result)
if got := fixture.postNames(); !reflect.DeepEqual(got, []string{"app.bin"}) {
t.Fatalf("POST names = %v", got)
}
@@ -163,17 +167,100 @@ func TestUploadMissingPreflightConflictPreventsEveryWrite(t *testing.T) {
if got, want := names(result.Pending), []string{"other.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("pending = %v, want %v", got, want)
}
assertAssetCategoriesDisjoint(t, result)
}
func TestUploadMissingKeepsLaterPendingAfterAmbiguousMissingUpload(t *testing.T) {
fixture := &uploadFixture{files: make(map[string][]byte), dropAckMissingName: "alpha.bin"}
server := httptest.NewServer(fixture)
defer server.Close()
client := testClient(server)
destination := testDestination(server.URL)
assets := localAssets(t, []string{"alpha.bin", "zeta.bin"})
initial, err := client.InspectAssets(context.Background(), destination, 10, assets)
if err != nil {
t.Fatal(err)
}
result, err := client.UploadMissing(context.Background(), destination, 10, initial)
if err == nil || result.State != DeliveryUnknown {
t.Fatalf("result = %#v, error = %v", result, err)
}
if got, want := names(result.Uncertain), []string{"alpha.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("uncertain = %v, want %v", got, want)
}
if got, want := names(result.Pending), []string{"zeta.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("pending = %v, want %v", got, want)
}
assertAssetCategoriesDisjoint(t, result)
if got, want := fixture.postNames(), []string{"alpha.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("POST names = %v, want %v", got, want)
}
}
func TestUploadMissingMovesReconciledConflictOutOfPending(t *testing.T) {
fixture := &uploadFixture{
files: make(map[string][]byte),
conflictName: "alpha.bin",
conflictDifferentContent: true,
}
server := httptest.NewServer(fixture)
defer server.Close()
client := testClient(server)
destination := testDestination(server.URL)
assets := localAssets(t, []string{"alpha.bin", "zeta.bin"})
initial, err := client.InspectAssets(context.Background(), destination, 10, assets)
if err != nil {
t.Fatal(err)
}
result, err := client.UploadMissing(context.Background(), destination, 10, initial)
if err == nil || result.State != DeliveryConflict {
t.Fatalf("result = %#v, error = %v", result, err)
}
if got, want := names(result.Conflicting), []string{"alpha.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("conflicting = %v, want %v", got, want)
}
if got, want := names(result.Pending), []string{"zeta.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("pending = %v, want %v", got, want)
}
assertAssetCategoriesDisjoint(t, result)
}
func TestUploadMissingTreatsUnresolvedNameConflictAsConflict(t *testing.T) {
fixture := &uploadFixture{files: make(map[string][]byte), conflictMissingName: "alpha.bin"}
server := httptest.NewServer(fixture)
defer server.Close()
client := testClient(server)
destination := testDestination(server.URL)
assets := localAssets(t, []string{"alpha.bin", "zeta.bin"})
initial, err := client.InspectAssets(context.Background(), destination, 10, assets)
if err != nil {
t.Fatal(err)
}
result, err := client.UploadMissing(context.Background(), destination, 10, initial)
if err == nil || result.State != DeliveryConflict {
t.Fatalf("result = %#v, error = %v", result, err)
}
if got, want := names(result.Conflicting), []string{"alpha.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("conflicting = %v, want %v", got, want)
}
if got, want := names(result.Pending), []string{"zeta.bin"}; !reflect.DeepEqual(got, want) {
t.Fatalf("pending = %v, want %v", got, want)
}
assertAssetCategoriesDisjoint(t, result)
}
type uploadFixture struct {
mu sync.Mutex
files map[string][]byte
posts []string
failName string
conflictName string
dropAckName string
failListAfter int
listCount int
mu sync.Mutex
files map[string][]byte
posts []string
failName string
conflictName string
conflictDifferentContent bool
conflictMissingName string
dropAckName string
dropAckMissingName string
failListAfter int
listCount int
}
func (fixture *uploadFixture) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -254,6 +341,30 @@ func (fixture *uploadFixture) receiveUpload(w http.ResponseWriter, r *http.Reque
http.Error(w, "write failed", http.StatusBadRequest)
return
}
if name == fixture.dropAckMissingName {
fixture.mu.Unlock()
hijacker, ok := w.(http.Hijacker)
if !ok {
http.Error(w, "hijacking unavailable", http.StatusInternalServerError)
return
}
connection, _, err := hijacker.Hijack()
if err == nil {
_ = connection.Close()
}
return
}
if name == fixture.conflictMissingName {
fixture.mu.Unlock()
w.WriteHeader(http.StatusConflict)
return
}
if name == fixture.conflictName && fixture.conflictDifferentContent {
fixture.files[name] = []byte("conflicting bytes")
fixture.mu.Unlock()
w.WriteHeader(http.StatusConflict)
return
}
fixture.files[name] = append([]byte(nil), content...)
fixture.mu.Unlock()
if name == fixture.conflictName {
@@ -318,6 +429,29 @@ func names(identities []AssetIdentity) []string {
return result
}
func assertAssetCategoriesDisjoint(t *testing.T, result UploadResult) {
t.Helper()
groups := []struct {
name string
identities []AssetIdentity
}{
{name: "matching", identities: result.Matching},
{name: "uploaded", identities: result.Uploaded},
{name: "pending", identities: result.Pending},
{name: "conflicting", identities: result.Conflicting},
{name: "uncertain", identities: result.Uncertain},
}
seen := make(map[string]string)
for _, group := range groups {
for _, item := range group.identities {
if previous, ok := seen[item.Filename]; ok {
t.Errorf("asset %q appears in both %s and %s", item.Filename, previous, group.name)
}
seen[item.Filename] = group.name
}
}
}
func urlPathEscape(value string) string {
return (&url.URL{Path: value}).EscapedPath()
}