From ccbf39ed06aeba144441fcbbdd20062a68218b87 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 23 Jun 2026 14:57:46 -0400 Subject: [PATCH 1/2] fix: complete hotfix-rejoin cleanup for published releases Signed-off-by: Joshua Temple --- .../hotfix-rejoin-prerelease-supersede.yaml | 153 ++++++++++++++++ internal/promote/finalize.go | 22 ++- internal/promote/rejoin.go | 44 ++++- internal/promote/rejoin_cleanup_test.go | 167 ++++++++++++++++++ internal/release/release.go | 65 +++++-- internal/release/release_test.go | 127 ++++++++++++- 6 files changed, 549 insertions(+), 29 deletions(-) create mode 100644 e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml create mode 100644 internal/promote/rejoin_cleanup_test.go diff --git a/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml b/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml new file mode 100644 index 00000000..1ea671b5 --- /dev/null +++ b/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml @@ -0,0 +1,153 @@ +name: "Hotfix Rejoin After Prerelease Supersede" +description: | + Regression guard for the rejoin cleanup of a hotfix that landed on the + prerelease environment. The prerelease env is the second-from-top env + (staging in [dev, test, staging, prod]); a hotfix there promotes its release + object to a GitHub prerelease (draft:false). A later superseding promote that + carries the recorded patch on trunk must then rejoin the env to trunk and + clean up its integration branch, hotfix tag, and (on real GitHub) the now + non-draft hotfix release object. Before the fix, the cleanup aborted on the + non-draft release and wedged the rejoin. + + The act/gitea backend skips GitHub release-object operations (no release API), + so the release-object delete is a no-op here; the observable rejoin contract + this scenario exercises is the state rejoin, the env/ branch deletion, and + the hotfix tag cleanup. The real-GitHub release-object delete is covered by the + validation fleet and the internal/promote + internal/release unit tests. + + Because the recorded patch is the original trunk commit SHA, landing that same + trunk ancestor on dev and cascading it into staging satisfies containment and + triggers the rejoin. + +config: + trunk_branch: main + environments: [dev, test, staging, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: + - name: deploy-dev + workflow: deploy.yaml + triggers: ["src/**"] + - name: deploy-test + workflow: deploy.yaml + triggers: ["src/**"] + - name: deploy-staging + workflow: deploy.yaml + triggers: ["src/**"] + - name: deploy-prod + workflow: deploy.yaml + triggers: ["src/**"] + +steps: + - name: "Initial commit" + action: commit + commit: + message: "feat: add app" + files: + src/app.go: | + package main + func main() {} + + - name: "Orchestrate trunk into dev" + action: orchestrate + + - name: "Promote to establish a staging baseline" + action: promote + promote: + mode: default + + - name: "Promote again to carry the baseline toward staging" + action: promote + promote: + mode: default + + - name: "Promote once more so staging shares the baseline" + action: promote + promote: + mode: default + expect: + state: + staging: + version: "v0.1.0-rc.1" + + - name: "Commit a trunk fix to hotfix into staging" + action: commit + commit: + message: "fix: patch for staging env" + files: + src/fix.go: | + package main + func patch() {} + + - name: "Plan hotfix for the prerelease env (staging)" + action: hotfix_plan + hotfix_plan: + target_env: staging + commit_ref: commit2 + # Plan only: the harness's hotfix_apply step performs the real cherry-pick + # and PR via the gitea API. Running the workflow's apply job here would call + # the GitHub gh CLI, which is absent from the act runner image. + dry_run: true + + - name: "Apply hotfix onto env/staging" + action: hotfix_apply + hotfix_apply: + target_env: staging + commit_ref: commit2 + + - name: "Merge the hotfix pull request" + action: merge_pr + merge_pr: + label: cascade-hotfix + + - name: "Finalize hotfix; staging diverges and mints a hotfix tag" + action: hotfix_merged + hotfix_merged: + target_env: staging + expect: + state: + staging: + ref: "env/staging" + tags: + exist: ["v0.1.0-rc.1.hotfix.1"] + + # Land a normal trunk commit. Trunk now contains commit2 (the recorded patch) + # as an ancestor, so a promotion of any later trunk SHA into staging will + # satisfy patch containment and end the divergence. + - name: "Advance trunk normally past the patch" + action: commit + commit: + message: "chore: trunk advance" + files: + src/advance.go: | + package main + func advance() {} + + - name: "Orchestrate so dev carries the patch's trunk ancestor" + action: orchestrate + + # Rejoin: a targeted cascade of dev into the diverged staging env, where the + # incoming SHA contains every recorded patch, supersedes the hotfix and ends + # the divergence. The cleanup clears staging's divergence fields, deletes + # env/staging on the remote, and removes the hotfix tag (and, on real GitHub, + # the now non-draft hotfix release object). A targeted cascade is used so only + # the rejoin leg runs; default mode would also queue the staging-to-prod leg, + # which sources FROM the still-diverged staging env and trips the + # diverged-source guard before the rejoin can clear it. prod is untouched. + - name: "Promote a containing SHA into staging triggers rejoin" + action: promote + promote: + mode: cascade + target: staging + expect: + state: + staging: + cleared: [ref, base_sha, patches] + prod: + unchanged: true + branches: + deleted: ["env/staging"] + tags: + deleted: ["v0.1.0-rc.1.hotfix.1"] diff --git a/internal/promote/finalize.go b/internal/promote/finalize.go index e1733da7..42150e8a 100644 --- a/internal/promote/finalize.go +++ b/internal/promote/finalize.go @@ -124,25 +124,33 @@ func (f *Finalizer) Run() error { // runLifecycleCleanup performs the divergence-end side effects for every env // that rejoined trunk during this finalization. It runs only after the manifest // is persisted, so the source of truth is updated before any branch, tag, or -// draft is removed; a cleanup failure then leaves the manifest correct and the -// operation re-runnable. When no env rejoined (the common, non-diverged case) -// this is a no-op and the injected cleaner is never called. +// release object is removed. +// +// Cleanup is best-effort and never aborts the finalize: the objects it removes +// (integration branch, hotfix tags and release objects) are disposable +// superseded artifacts, and every individual delete is idempotent, so a transient +// failure on one env must not strand the others or wedge an already-persisted +// state write. A failure on one env is logged loudly and cleanup continues with +// the rest; hard-fail is reserved for the state write, which Run performs before +// reaching here. When no env rejoined (the common, non-diverged case) this is a +// no-op and the injected cleaner is never called. func (f *Finalizer) runLifecycleCleanup() error { for _, ev := range f.pendingRejoins { if ev.rollbackOrigin { // A manual rollback creates no integration branch, hotfix tags, or - // release drafts. The divergence fields were already cleared above, + // release objects. The divergence fields were already cleared above, // so the rejoin is complete with no side effects to undo. continue } if err := f.cleaner.DeleteEnvBranch(ev.env); err != nil { - return fmt.Errorf("rejoin cleanup for %s: %w", ev.env, err) + fmt.Printf("Warning: rejoin cleanup for %s: deleting integration branch: %v\n", ev.env, err) } if err := f.cleaner.CleanHotfixReleases(CleanReleasesRequest{ Environment: ev.env, BaseVersion: ev.baseVersion, + SHA: ev.sha, }); err != nil { - return fmt.Errorf("rejoin cleanup for %s: %w", ev.env, err) + fmt.Printf("Warning: rejoin cleanup for %s: cleaning hotfix releases: %v\n", ev.env, err) } } return nil @@ -174,6 +182,7 @@ func (f *Finalizer) updateState() { // env held while diverged, captured here before it is overwritten. wasDiverged := state.IsDiverged() priorVersion := state.Version + priorSHA := state.SHA // When an auto-committing callback ran, overrideSHA holds the // post-callback HEAD; use it so the recorded state points at the @@ -208,6 +217,7 @@ func (f *Finalizer) updateState() { f.pendingRejoins = append(f.pendingRejoins, rejoinEvent{ env: promo.Environment, baseVersion: priorVersion, + sha: priorSHA, rollbackOrigin: rollbackOrigin, }) } diff --git a/internal/promote/rejoin.go b/internal/promote/rejoin.go index 81818b98..e67f8d4c 100644 --- a/internal/promote/rejoin.go +++ b/internal/promote/rejoin.go @@ -16,6 +16,11 @@ import ( type CleanReleasesRequest struct { Environment string BaseVersion string + // SHA is the commit the environment pointed at while diverged (the hotfix + // merge SHA). It is passed to the release lookup as a fallback so a hotfix + // release whose tag was already deleted by a prior partial run can still be + // resolved by its target_commitish and removed, rather than leaking. + SHA string } // LifecycleCleaner performs the side effects of ending a divergence: deleting @@ -63,6 +68,10 @@ func WithLifecycleCleaner(c LifecycleCleaner) FinalizeOption { type rejoinEvent struct { env string baseVersion string + // sha is the commit the env pointed at while diverged (its hotfix merge SHA), + // passed through to the release cleanup as a lookup fallback so a hotfix + // release can still be resolved if its tag was removed by a prior partial run. + sha string // rollbackOrigin is true when the env diverged via a manual rollback rather // than a hotfix integration branch. The rejoin cleanup skips integration // branch and hotfix release deletion in that case, since a rollback creates @@ -120,10 +129,16 @@ func (c *gitReleaseCleaner) DeleteEnvBranch(env string) error { return nil } -// CleanHotfixReleases deletes the hotfix tags for the prior base version and the -// matching draft release objects. Tag and draft deletion is best-effort per -// item so one stale object does not block the others; the first hard error is -// returned. +// CleanHotfixReleases deletes the hotfix release objects for the prior base +// version and then the matching tags. The release object is removed FIRST and the +// tag is deleted only when the release delete succeeded (or the release was +// already gone): a failed release delete leaves the tag intact so a rerun can +// still resolve the release object by tag rather than orphaning a now-tagless +// release. The hotfix release may be a non-draft prerelease (the prerelease env +// promotes its hotfix release to draft:false), so AllowPublishedDelete is set; +// the recorded SHA is passed as a lookup fallback for a tag that a prior partial +// run already removed. Cleanup is best-effort per item so one stale object does +// not block the others; the first hard error is returned. func (c *gitReleaseCleaner) CleanHotfixReleases(req CleanReleasesRequest) error { tags, err := c.listTags() if err != nil { @@ -133,15 +148,26 @@ func (c *gitReleaseCleaner) CleanHotfixReleases(req CleanReleasesRequest) error var firstErr error for _, tag := range hotfixTags { - // Remove the draft release object for the hotfix tag, then the tag. + // Delete the release object first. Only delete the tag if that succeeded, + // so a failed release delete leaves the tag for a rerun to retry. + releaseDeleted := true if c.releaseMgr != nil { if _, err := c.releaseMgr.Manage(release.Options{ - Action: release.ActionDelete, - Tag: tag, - }); err != nil && firstErr == nil { - firstErr = fmt.Errorf("deleting hotfix release %s: %w", tag, err) + Action: release.ActionDelete, + Tag: tag, + SHA: req.SHA, + AllowPublishedDelete: true, + }); err != nil { + releaseDeleted = false + if firstErr == nil { + firstErr = fmt.Errorf("deleting hotfix release %s: %w", tag, err) + } } } + if !releaseDeleted { + // Leave the tag so the next run can still resolve the release by tag. + continue + } if err := c.deleteTag(c.remote, tag); err != nil && firstErr == nil { firstErr = fmt.Errorf("deleting hotfix tag %s: %w", tag, err) } diff --git a/internal/promote/rejoin_cleanup_test.go b/internal/promote/rejoin_cleanup_test.go new file mode 100644 index 00000000..728e80e8 --- /dev/null +++ b/internal/promote/rejoin_cleanup_test.go @@ -0,0 +1,167 @@ +package promote + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stablekernel/cascade/internal/release" + "github.com/stretchr/testify/require" +) + +// newStubbedCleaner builds a gitReleaseCleaner whose release operations target the +// given httptest server and whose git tag list/delete are replaced by the supplied +// stubs, so the cleanup's release-then-tag ordering can be exercised without a real +// remote. +func newStubbedCleaner(serverURL string, tags []string, deleteTag func(remote, name string) error) *gitReleaseCleaner { + mgr := release.NewManagerWithURL("owner/repo", "token", serverURL) + return &gitReleaseCleaner{ + remote: "origin", + releaseMgr: mgr, + listTags: func() ([]string, error) { return tags, nil }, + deleteTag: deleteTag, + } +} + +// TestCleanHotfixReleases_PublishedRelease_Completes covers F1: a hotfix release +// that was promoted to a prerelease (non-draft) must still be deletable by the +// rejoin cleanup, which the general delete guard would otherwise reject. +func TestCleanHotfixReleases_PublishedRelease_Completes(t *testing.T) { + const tag = "v1.4.0-rc.2.hotfix.1" + releaseDeleted := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases/tags/"+tag): + w.WriteHeader(http.StatusOK) + // Non-draft: the prerelease promotion patched draft:false. + _ = json.NewEncoder(w).Encode(release.GitHubRelease{ID: 42, TagName: tag, Draft: false, Prerelease: true}) + case r.Method == http.MethodDelete && strings.Contains(r.URL.Path, "/releases/"): + releaseDeleted = true + w.WriteHeader(http.StatusNoContent) + default: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{}) + } + })) + defer server.Close() + + tagDeleted := false + cleaner := newStubbedCleaner(server.URL, []string{tag}, func(remote, name string) error { + tagDeleted = true + return nil + }) + + err := cleaner.CleanHotfixReleases(CleanReleasesRequest{Environment: "test", BaseVersion: "v1.4.0-rc.2"}) + require.NoError(t, err, "a non-draft hotfix release must be deletable by rejoin cleanup") + require.True(t, releaseDeleted, "the non-draft hotfix release object must be deleted") + require.True(t, tagDeleted, "the hotfix tag must be deleted after a successful release delete") +} + +// TestCleanHotfixReleases_ReleaseDeleteError_KeepsTag covers F2: when the release +// delete fails, the matching tag must NOT be deleted, so a rerun can still resolve +// the release object by tag rather than orphaning a tagless release. +func TestCleanHotfixReleases_ReleaseDeleteError_KeepsTag(t *testing.T) { + const tag = "v1.4.0-rc.2.hotfix.1" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases/tags/"+tag): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(release.GitHubRelease{ID: 42, TagName: tag, Draft: true}) + case r.Method == http.MethodDelete && strings.Contains(r.URL.Path, "/releases/"): + // Transient server error on the release delete. + w.WriteHeader(http.StatusInternalServerError) + default: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{}) + } + })) + defer server.Close() + + tagDeleted := false + cleaner := newStubbedCleaner(server.URL, []string{tag}, func(remote, name string) error { + tagDeleted = true + return nil + }) + + err := cleaner.CleanHotfixReleases(CleanReleasesRequest{Environment: "test", BaseVersion: "v1.4.0-rc.2"}) + require.Error(t, err, "a failed release delete must surface as an error") + require.False(t, tagDeleted, "the tag must be preserved when the release delete fails, so a rerun can resolve it") +} + +// TestCleanHotfixReleases_AlreadyDeleted_Idempotent covers the rerun path: when the +// release object is already gone (404) the cleanup deletes the tag and succeeds, so +// a second run after a partial failure converges. +func TestCleanHotfixReleases_AlreadyDeleted_Idempotent(t *testing.T) { + const tag = "v1.4.0-rc.2.hotfix.1" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases/tags/"+tag): + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode([]release.GitHubRelease{}) + default: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{}) + } + })) + defer server.Close() + + tagDeleted := false + cleaner := newStubbedCleaner(server.URL, []string{tag}, func(remote, name string) error { + tagDeleted = true + return nil + }) + + err := cleaner.CleanHotfixReleases(CleanReleasesRequest{Environment: "test", BaseVersion: "v1.4.0-rc.2"}) + require.NoError(t, err, "an already-deleted release must not error on rerun") + require.True(t, tagDeleted, "the orphaned tag must still be cleaned up on rerun") +} + +// failingTagCleaner records that DeleteEnvBranch was called and lets the test fail +// the hotfix-release cleanup of a specific env, to exercise the half-completed +// multi-env rejoin rerun (L1). +type failingTagCleaner struct { + deletedBranches []string + cleaned []string + failOnEnv string +} + +func (c *failingTagCleaner) DeleteEnvBranch(env string) error { + c.deletedBranches = append(c.deletedBranches, env) + return nil +} + +func (c *failingTagCleaner) CleanHotfixReleases(req CleanReleasesRequest) error { + if req.Environment == c.failOnEnv { + return fmt.Errorf("transient cleanup failure for %s", req.Environment) + } + c.cleaned = append(c.cleaned, req.Environment) + return nil +} + +// TestRunLifecycleCleanup_BestEffort_DoesNotStrandOtherEnvs covers L1: when one +// env's hotfix-release cleanup fails during a multi-env rejoin, the finalize must +// not abort. The remaining envs must still be cleaned, and Run must succeed so the +// already-persisted state is not wedged behind a disposable-object cleanup failure. +// Cleanup of disposable objects is best-effort; only state writes hard-fail. +func TestRunLifecycleCleanup_BestEffort_DoesNotStrandOtherEnvs(t *testing.T) { + cleaner := &failingTagCleaner{failOnEnv: "test"} + f := &Finalizer{ + cleaner: cleaner, + pendingRejoins: []rejoinEvent{ + {env: "test", baseVersion: "v1.4.0-rc.2.hotfix.1"}, + {env: "uat", baseVersion: "v1.3.0-rc.5.hotfix.1"}, + }, + } + + err := f.runLifecycleCleanup() + require.NoError(t, err, "a cleanup failure for one env must not abort finalize") + + require.Contains(t, cleaner.deletedBranches, "test") + require.Contains(t, cleaner.deletedBranches, "uat", "the second env's branch must still be cleaned after the first env's cleanup error") + require.Contains(t, cleaner.cleaned, "uat", "the second env's hotfix releases must still be cleaned") +} diff --git a/internal/release/release.go b/internal/release/release.go index d1d80274..5144725c 100644 --- a/internal/release/release.go +++ b/internal/release/release.go @@ -95,6 +95,14 @@ type Options struct { // directly instead of re-discovering the release by tag, eliminating the // eventual-consistency window between draft creation and the list endpoint. KnownReleaseID int64 + // AllowPublishedDelete permits ActionDelete to remove a non-draft (published + // or prerelease) release. It exists ONLY for hotfix-rejoin cleanup, where the + // release is a superseded intermediate artifact: a hotfix on the prerelease + // env is promoted to a GitHub prerelease (draft:false), so by the time a + // later promote rejoins the env the hotfix release is non-draft and the + // general guard would otherwise wedge the cleanup. Normal publish/promote + // delete paths leave this false so a real published release is still protected. + AllowPublishedDelete bool } // ValidateAction checks if the action is valid @@ -621,6 +629,18 @@ func (m *Manager) publish(opts Options) (*Result, error) { return nil, err } + // Re-run convergence: a publish that already patched the release to the semver + // tag and cleaned up the rc tags will not resolve by the rc tag on a rerun. + // Fall back to the semver tag so a partially completed publish (semver tag + // created, rc cleanup done, but the manifest write failed) still finds the + // already-published object and converges instead of erroring. + if existing == nil && searchTag != opts.Tag { + existing, err = m.findRelease(opts.Tag, opts.SHA) + if err != nil { + return nil, err + } + } + if existing == nil { return nil, fmt.Errorf("no release found for tag %s", searchTag) } @@ -671,8 +691,11 @@ func (m *Manager) delete(opts Options) (*Result, error) { return &Result{}, nil } - if !existing.Draft { - return nil, fmt.Errorf("cannot delete published release %s", opts.Tag) + // Guard against destroying a real published release. The hotfix-rejoin + // cleanup opts in via AllowPublishedDelete because the release it removes is a + // superseded hotfix prerelease (non-draft), not a release a user cares about. + if !existing.Draft && !opts.AllowPublishedDelete { + return nil, fmt.Errorf("cannot delete non-draft release %s without AllowPublishedDelete", opts.Tag) } result := &Result{ @@ -792,26 +815,42 @@ func (m *Manager) findReleaseByTagOrSHA(tag, sha string) (*GitHubRelease, error) } _ = resp.Body.Close() - // Find release matching the tag or SHA (prefer draft over published for updates) - var found *GitHubRelease + // Resolve the best match in priority order so a stale draft sharing a + // target_commitish cannot win over the intended release: + // 1. a draft whose tag matches exactly (strongest signal), + // 2. a draft matched only by SHA (fallback for a tagless object whose + // tag a prior partial run already removed), + // 3. any non-draft (published/prerelease) match. + // An exact tag match is preferred over a SHA-only match because two drafts + // can share a target_commitish; the tag disambiguates. + var shaOnlyDraft *GitHubRelease + var nonDraft *GitHubRelease for i := range releases { - // Match by tag_name, name, or SHA (target_commitish) tagMatch := tag != "" && (releases[i].TagName == tag || releases[i].Name == tag) shaMatch := sha != "" && releases[i].TargetCommitish == sha - - if tagMatch || shaMatch { - if releases[i].Draft { - // Prefer draft - return immediately + if !tagMatch && !shaMatch { + continue + } + if releases[i].Draft { + if tagMatch { + // Strongest signal - return immediately. return &releases[i], nil } - if found == nil { - found = &releases[i] + if shaOnlyDraft == nil { + shaOnlyDraft = &releases[i] } + continue + } + if nonDraft == nil { + nonDraft = &releases[i] } } - if found != nil { - return found, nil + if shaOnlyDraft != nil { + return shaOnlyDraft, nil + } + if nonDraft != nil { + return nonDraft, nil } // found == nil: list returned nothing matching - may be a consistency // window; retry on next iteration unless this was the last attempt. diff --git a/internal/release/release_test.go b/internal/release/release_test.go index 4aae3331..636850fe 100644 --- a/internal/release/release_test.go +++ b/internal/release/release_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -334,7 +335,131 @@ func TestManager_Delete_PublishedRelease(t *testing.T) { }) assert.Error(t, err) - assert.Contains(t, err.Error(), "cannot delete published release") + assert.Contains(t, err.Error(), "cannot delete non-draft release") +} + +// TestManager_Delete_PublishedRelease_AllowPublished verifies that the scoped +// AllowPublishedDelete opt-in lets a non-draft (published or prerelease) release +// be deleted. This path exists only for hotfix-rejoin cleanup, where the release +// is a superseded intermediate artifact; the general guard (exercised by +// TestManager_Delete_PublishedRelease) stays in force without the flag. +func TestManager_Delete_PublishedRelease_AllowPublished(t *testing.T) { + deleted := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _ = json.NewEncoder(w).Encode(GitHubRelease{ + ID: 790, + TagName: "v1.0.0-rc.2.hotfix.1", + Draft: false, // non-draft (prerelease promoted) + }) + return + } + assert.Equal(t, http.MethodDelete, r.Method) + deleted = true + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + manager := &Manager{ + client: server.Client(), + baseURL: server.URL, + token: "test-token", + repo: "owner/repo", + } + + result, err := manager.Manage(Options{ + Action: ActionDelete, + Tag: "v1.0.0-rc.2.hotfix.1", + AllowPublishedDelete: true, + }) + + require.NoError(t, err) + assert.Equal(t, int64(790), result.ReleaseID) + assert.True(t, deleted, "the non-draft release must be deleted when AllowPublishedDelete is set") +} + +// TestFindReleaseByTagOrSHA_PrefersExactTagOverStaleSHADraft covers L4: when a +// stale draft shares a target_commitish with the intended draft, the lookup must +// prefer the draft whose tag matches exactly rather than returning the first +// SHA-only match. +func TestFindReleaseByTagOrSHA_PrefersExactTagOverStaleSHADraft(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Direct by-tag endpoint misses so the list-scan path is exercised. + if strings.Contains(r.URL.Path, "/releases/tags/") { + w.WriteHeader(http.StatusNotFound) + return + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode([]GitHubRelease{ + // Stale draft sharing the SHA but a different (older) tag, listed first. + {ID: 1, TagName: "v1.0.0-rc.0", TargetCommitish: "deadbeef", Draft: true}, + // The intended draft: exact tag match on the same SHA. + {ID: 2, TagName: "v1.0.0-rc.1", TargetCommitish: "deadbeef", Draft: true}, + }) + })) + defer server.Close() + + manager := &Manager{ + client: server.Client(), + baseURL: server.URL, + token: "test-token", + repo: "owner/repo", + sleepFn: func(time.Duration) {}, + } + + got, err := manager.findRelease("v1.0.0-rc.1", "deadbeef") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, int64(2), got.ID, "the exact tag match must win over a stale SHA-only draft") +} + +// TestManager_Publish_RerunFallsBackToSemverTag covers L3: a publish rerun after a +// partial publish (the rc tag was already cleaned up) must resolve the +// already-published release by its semver tag and converge instead of erroring. +func TestManager_Publish_RerunFallsBackToSemverTag(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases/tags/v1.0.0-rc.5"): + // rc tag already cleaned up by the first (partial) publish. + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases/tags/v1.0.0"): + // Already published under the semver tag. + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(GitHubRelease{ID: 99, TagName: "v1.0.0", Draft: false}) + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode([]GitHubRelease{}) + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/git/refs/tags"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode([]struct { + Ref string `json:"ref"` + }{}) + case r.Method == http.MethodPatch: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(GitHubRelease{ID: 99, TagName: "v1.0.0"}) + default: + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{}) + } + })) + defer server.Close() + + manager := &Manager{ + client: server.Client(), + baseURL: server.URL, + token: "test-token", + repo: "owner/repo", + sleepFn: func(time.Duration) {}, + } + + result, err := manager.Manage(Options{ + Action: ActionPublish, + Tag: "v1.0.0", + DeleteTag: "v1.0.0-rc.5", + SHA: "abc123", + }) + require.NoError(t, err, "publish rerun must converge by resolving the semver tag") + assert.Equal(t, int64(99), result.ReleaseID) } func TestManager_Delete_NotFound(t *testing.T) { From 47ae45f4ec0a9174b4d51990aaf935355698d6c5 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 23 Jun 2026 15:49:10 -0400 Subject: [PATCH 2/2] test(e2e): assert rejoin via on-backend signals in the supersede scenario Signed-off-by: Joshua Temple --- .../hotfix-rejoin-prerelease-supersede.yaml | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml b/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml index 1ea671b5..57400ae4 100644 --- a/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml +++ b/e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml @@ -10,10 +10,13 @@ description: | non-draft release and wedged the rejoin. The act/gitea backend skips GitHub release-object operations (no release API), - so the release-object delete is a no-op here; the observable rejoin contract - this scenario exercises is the state rejoin, the env/ branch deletion, and - the hotfix tag cleanup. The real-GitHub release-object delete is covered by the - validation fleet and the internal/promote + internal/release unit tests. + and the hotfix tag is materialized only through that same release-object path, + so neither the tag nor its later delete is observable here. The observable + rejoin contract this scenario exercises is the recorded divergence state (the + minted hotfix version on env/) and, after rejoin, the cleared state and + the env/ branch deletion. The real-GitHub hotfix tag and release-object + create/delete are covered by the validation fleet and the internal/promote + + internal/release unit tests. Because the recorded patch is the original trunk commit SHA, landing that same trunk ancestor on dev and cascading it into staging satisfies containment and @@ -70,7 +73,7 @@ steps: expect: state: staging: - version: "v0.1.0-rc.1" + version: "v0.1.0-rc.0" - name: "Commit a trunk fix to hotfix into staging" action: commit @@ -102,16 +105,21 @@ steps: merge_pr: label: cascade-hotfix - - name: "Finalize hotfix; staging diverges and mints a hotfix tag" + - name: "Finalize hotfix; staging diverges and mints a hotfix version" action: hotfix_merged hotfix_merged: target_env: staging + # The hotfix tag (v0.1.0-rc.0.hotfix.1) is materialized only through the + # GitHub release-object path, which the act/gitea backend skips (no release + # API), so no such tag lands in gitea here. The observable divergence on this + # backend is the recorded state: env/staging ref and the minted hotfix + # version. The git-tag create/delete is covered by the internal/release and + # internal/promote rejoin tests. expect: state: staging: ref: "env/staging" - tags: - exist: ["v0.1.0-rc.1.hotfix.1"] + version: "v0.1.0-rc.0.hotfix.1" # Land a normal trunk commit. Trunk now contains commit2 (the recorded patch) # as an ancestor, so a promotion of any later trunk SHA into staging will @@ -149,5 +157,9 @@ steps: unchanged: true branches: deleted: ["env/staging"] - tags: - deleted: ["v0.1.0-rc.1.hotfix.1"] + # The hotfix tag and release-object delete run only on the GitHub + # release-object path, which the act/gitea backend skips, so no tag is + # materialized here to observe a delete against. The observable rejoin + # contract on this backend is the cleared divergence state and the deleted + # env/staging branch; the tag and release-object cleanup is covered by the + # internal/promote rejoin and internal/release unit tests.