fix: complete hotfix-rejoin cleanup for published releases

e2e/scenarios/hotfix/hotfix-rejoin-prerelease-supersede.yaml

@@ -0,0 +1,165 @@
+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),
+  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/<env>) and, after rejoin, the cleared state and
+  the env/<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
+  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.0"
+
+  - 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 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"
+          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
+  # 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"]
+      # 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.

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,
 				})
 			}

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)
 		}