diff --git a/delivery_test.go b/delivery_test.go index 2f430dd..287ebee 100644 --- a/delivery_test.go +++ b/delivery_test.go @@ -755,6 +755,33 @@ func (writer deliveryCancelWriter) Write(content []byte) (int, error) { return len(content), nil } +func TestDeliverExistingPageWithoutReleaseID(t *testing.T) { + repository := newTaggedDeliveryConsumer(t) + prebuilt, err := releasekit.Build(context.Background(), releasekit.BuildRequest{ + RepositoryDir: repository, Version: "v1.2.3", OutputDir: t.TempDir(), + Spec: releasekit.BuildSpec{MainPackage: "./cmd/app", BinaryName: "fixture", Targets: []releasekit.Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}}, + }) + if err != nil { + t.Fatal(err) + } + server, remote := newFacadeDeliveryServer(t, repository) + defer server.Close() + remote.update(func(data *facadeDeliveryData) { data.pages[data.tag] = true; data.omitReleaseID = true }) + request := releasekit.DeliveryRequest{RepositoryDir: repository, Product: "Fixture", Version: "v1.2.3", Destination: facadeDeliveryDestination(server.URL)} + page, err := releasekit.Deliver(context.Background(), request) + if err != nil || page.State != releasekit.DeliveryComplete { + t.Fatalf("page result = %#v, error = %v", page, err) + } + request.Prebuilt = &prebuilt + result, err := releasekit.Deliver(context.Background(), request) + if err == nil || result.State != releasekit.DeliveryNotAttempted || !strings.Contains(err.Error(), "release ID") { + t.Fatalf("asset result = %#v, error = %v", result, err) + } + if snapshot := remote.snapshot(); snapshot.writes != 0 { + t.Fatalf("unexpected hosted writes: %#v", snapshot) + } +} + type facadeDeliveryServer struct { mu sync.Mutex data facadeDeliveryData @@ -785,16 +812,17 @@ func (state *facadeDeliveryServer) snapshot() facadeDeliveryData { } type facadeDeliveryData struct { - t *testing.T - commit string - tag string - version string - noteBody string - pages map[string]bool - requests int - writes int - assets map[string][]byte - url string + t *testing.T + commit string + tag string + version string + noteBody string + pages map[string]bool + requests int + writes int + omitReleaseID bool + assets map[string][]byte + url string } func newFacadeDeliveryServer(t *testing.T, repository string) (*httptest.Server, *facadeDeliveryServer) { @@ -894,11 +922,15 @@ func (state *facadeDeliveryServer) writeRelease(w http.ResponseWriter) { // The request handler holds state.mu. data := &state.data w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ - "id": 10, "html_url": data.url + "/releases/10", "tag_name": data.tag, + release := map[string]any{ + "html_url": data.url + "/releases/10", "tag_name": data.tag, "target_commitish": data.commit, "name": "Fixture " + data.version, "body": data.noteBody, "draft": false, "prerelease": false, - }) + } + if !data.omitReleaseID { + release["id"] = 10 + } + _ = json.NewEncoder(w).Encode(release) } func mapKeys(values map[string][]byte) []string { diff --git a/docs/integrations/gitea.md b/docs/integrations/gitea.md index aeab738..3955907 100644 --- a/docs/integrations/gitea.md +++ b/docs/integrations/gitea.md @@ -54,6 +54,9 @@ is accepted by shared delivery policy only when it has that tag, the complete committed release note as its body, and stable non-draft state. Its existing title is retained. ReleaseKit does not edit a conflicting page. Slash-containing tags are escaped as one URL path segment. +Page-only delivery does not require a release ID in this lookup response. Asset +operations require a positive release ID; a missing or nonpositive ID is +reported as a malformed service observation before an asset request is made. If the page is absent, the adapter posts to `/api/v1/repos/{owner}/{repo}/releases` with the selected full tag and commit, diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8518ff9..7bd0b64 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -347,7 +347,9 @@ cancellation tests; run these with the race detector after they are coherent. ## Stage 8: Classify missing Gitea release IDs at asset use -**Status:** pending. +**Status:** complete. Offline Gitea and delivery package checks and public +page-only and prebuilt regressions pass, including missing, nonpositive, valid, +and wrong-type handles and not-attempted write evidence. **Files:** `internal/gitea/backend.go`, its tests, a focused root delivery test, and `docs/integrations/gitea.md`. diff --git a/internal/gitea/backend.go b/internal/gitea/backend.go index 3ccfd55..9314510 100644 --- a/internal/gitea/backend.go +++ b/internal/gitea/backend.go @@ -114,9 +114,9 @@ func (b *Backend) CreateRelease(ctx context.Context, request forge.CreateRelease } func (b *Backend) UploadAsset(ctx context.Context, handle any, filename string, content io.Reader) (forge.WriteEvidence, error) { - release, ok := handle.(releaseHandle) - if !ok || release.id <= 0 { - return forge.WriteEvidence{Disposition: forge.WriteNotAttempted}, &forge.Failure{Kind: forge.FailureInvalid, Err: errors.New("release handle was not created by the Gitea backend")} + release, err := assetReleaseHandle(handle) + if err != nil { + return forge.WriteEvidence{Disposition: forge.WriteNotAttempted}, err } if strings.TrimSpace(filename) == "" || content == nil { return forge.WriteEvidence{Disposition: forge.WriteNotAttempted}, &forge.Failure{Kind: forge.FailureInvalid, Err: errors.New("asset filename and content reader are required")} @@ -239,9 +239,9 @@ func (b *Backend) FindRelease(ctx context.Context, tag string) (forge.Release, e } func (b *Backend) ListAssets(ctx context.Context, handle any) ([]forge.Asset, error) { - release, ok := handle.(releaseHandle) - if !ok { - return nil, &forge.Failure{Kind: forge.FailureInvalid, Err: errors.New("release handle was not created by the Gitea backend")} + release, err := assetReleaseHandle(handle) + if err != nil { + return nil, err } _, apiRoot, err := b.destinationAPI("list release assets") if err != nil { @@ -262,6 +262,17 @@ func (b *Backend) ListAssets(ctx context.Context, handle any) ([]forge.Asset, er return assets, nil } +func assetReleaseHandle(handle any) (releaseHandle, error) { + release, ok := handle.(releaseHandle) + if !ok { + return releaseHandle{}, &forge.Failure{Kind: forge.FailureInvalid, Err: errors.New("release handle was not created by the Gitea backend")} + } + if release.id <= 0 { + return releaseHandle{}, &forge.Failure{Kind: forge.FailureMalformed, Err: errors.New("release response does not contain a positive release ID")} + } + return release, nil +} + func (b *Backend) OpenAsset(ctx context.Context, handle any) (io.ReadCloser, error) { asset, ok := handle.(assetHandle) if !ok { diff --git a/internal/gitea/backend_test.go b/internal/gitea/backend_test.go index 4338eaa..6024d73 100644 --- a/internal/gitea/backend_test.go +++ b/internal/gitea/backend_test.go @@ -64,6 +64,70 @@ func TestBackendFindReleaseDoesNotRequireWriteResponseFields(t *testing.T) { } } +func TestBackendAssetOperationsRequireObservedReleaseID(t *testing.T) { + for _, tc := range []struct { + name, body string + valid bool + }{ + {"missing", `{"tag_name":"v1.2.3"}`, false}, + {"zero", `{"id":0,"tag_name":"v1.2.3"}`, false}, + {"negative", `{"id":-1,"tag_name":"v1.2.3"}`, false}, + {"valid", `{"id":12,"tag_name":"v1.2.3"}`, true}, + } { + t.Run(tc.name, func(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/releases/tags/") { + _, _ = io.WriteString(w, tc.body) + return + } + requests++ + if r.Method == http.MethodGet { + _, _ = io.WriteString(w, `[]`) + return + } + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, `{"id":1,"name":"file"}`) + })) + defer server.Close() + backend := NewBackend(testDestination(server.URL), server.Client()) + release, err := backend.FindRelease(context.Background(), "v1.2.3") + if err != nil { + t.Fatal(err) + } + _, listErr := backend.ListAssets(context.Background(), release.Handle) + evidence, uploadErr := backend.UploadAsset(context.Background(), release.Handle, "file", strings.NewReader("data")) + if tc.valid { + if listErr != nil || uploadErr != nil || requests != 2 { + t.Fatalf("valid: list %v, upload %v, requests %d", listErr, uploadErr, requests) + } + return + } + for _, err := range []error{listErr, uploadErr} { + var failure *forge.Failure + if !errors.As(err, &failure) || failure.Kind != forge.FailureMalformed { + t.Fatalf("failure = %v", err) + } + } + if requests != 0 || evidence.Attempted || evidence.Disposition != forge.WriteNotAttempted { + t.Fatalf("requests = %d, evidence = %#v", requests, evidence) + } + }) + } + backend := NewBackend(Destination{}, nil) + _, listErr := backend.ListAssets(context.Background(), "wrong") + evidence, uploadErr := backend.UploadAsset(context.Background(), "wrong", "file", strings.NewReader("data")) + for _, err := range []error{listErr, uploadErr} { + var failure *forge.Failure + if !errors.As(err, &failure) || failure.Kind != forge.FailureInvalid { + t.Fatalf("wrong handle failure = %v", err) + } + } + if evidence.Attempted || evidence.Disposition != forge.WriteNotAttempted { + t.Fatalf("wrong handle evidence = %#v", evidence) + } +} + func TestBackendListsMetadataAndOpensCallerOwnedAssetStream(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if got, want := r.Header.Get("Authorization"), "token fixture-secret-token"; got != want {