From 7007e931a67938fe48e68739bfd3701573d5ee4b Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 10 Jun 2026 23:17:55 -0400 Subject: [PATCH 1/3] refactor(git): extract shared rebase-retry push helper Promote and hotfix finalize both write manifest state to trunk with the same commit-and-push-with-rebase-retry sequence. Extract it into internal/git as CommitAndPushWithRetry and ListTags, and have promote's CommitAndPush delegate so behavior stays identical. Signed-off-by: Joshua Temple --- internal/git/git.go | 46 +++++++++++++++++++++++++++++++++++++ internal/promote/promote.go | 37 ++++------------------------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/internal/git/git.go b/internal/git/git.go index f92e8619..a3ce46c8 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -6,6 +6,7 @@ import ( "fmt" "os/exec" "strings" + "time" ) // GetChangedFiles returns the list of files changed between two commits @@ -160,6 +161,51 @@ func GetLatestTag(prefix string) (string, string, error) { return latestTag, strings.TrimSpace(string(output)), nil } +// ListTags returns every tag in the repository. It returns an empty slice when +// the repository has no tags. +func ListTags() ([]string, error) { + cmd := exec.Command("git", "tag", "-l") + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git tag -l: %w", err) + } + return parseLines(output), nil +} + +// CommitAndPushWithRetry stages filePath, commits it with message, and pushes +// to the current branch's upstream, retrying the push up to three times behind a +// pull --rebase. A "nothing to commit" state is treated as success (no-op). This +// is the manifest state-write path shared by promote and hotfix finalize: an +// API-created commit on real GitHub goes through a different path, so this is the +// plain-git fallback used when committing locally. +func CommitAndPushWithRetry(filePath, message string) error { + cmd := exec.Command("git", "add", filePath) + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("git add failed: %s: %w", string(out), err) + } + + cmd = exec.Command("git", "commit", "-m", message) + if out, err := cmd.CombinedOutput(); err != nil { + if strings.Contains(string(out), "nothing to commit") { + return nil + } + return fmt.Errorf("git commit failed: %s: %w", string(out), err) + } + + for i := 0; i < 3; i++ { + cmd = exec.Command("git", "push") + if _, err := cmd.CombinedOutput(); err == nil { + return nil + } + + cmd = exec.Command("git", "pull", "--rebase") + _, _ = cmd.CombinedOutput() // ignore error - best effort + time.Sleep(2 * time.Second) + } + + return fmt.Errorf("git push failed after 3 retries") +} + // CommitAndPush stages a file, commits with the given message, and pushes to origin. func CommitAndPush(filePath, message string) error { // Stage the file diff --git a/internal/promote/promote.go b/internal/promote/promote.go index 12b7a588..63b2c1be 100644 --- a/internal/promote/promote.go +++ b/internal/promote/promote.go @@ -4,11 +4,11 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "strings" "time" "github.com/stablekernel/cascade/internal/config" + "github.com/stablekernel/cascade/internal/git" "gopkg.in/yaml.v3" ) @@ -684,38 +684,11 @@ func (p *Promoter) cascadePromotion(target string) (*PromotionResult, error) { return result, nil } -// CommitAndPush commits the state change and pushes to remote +// CommitAndPush commits the state change and pushes to remote. It delegates to +// the shared git rebase-retry helper so promote and hotfix finalize write +// manifest state identically. func (p *Promoter) CommitAndPush(message string) error { - // Git add - cmd := exec.Command("git", "add", p.configPath) - if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("git add failed: %s: %w", string(out), err) - } - - // Git commit - cmd = exec.Command("git", "commit", "-m", message) - if out, err := cmd.CombinedOutput(); err != nil { - // Check if nothing to commit - if strings.Contains(string(out), "nothing to commit") { - return nil - } - return fmt.Errorf("git commit failed: %s: %w", string(out), err) - } - - // Git push with retry - for i := 0; i < 3; i++ { - cmd = exec.Command("git", "push") - if _, err := cmd.CombinedOutput(); err == nil { - return nil - } - - // Pull and retry - cmd = exec.Command("git", "pull", "--rebase") - _, _ = cmd.CombinedOutput() // ignore error - best effort - time.Sleep(2 * time.Second) - } - - return fmt.Errorf("git push failed after 3 retries") + return git.CommitAndPushWithRetry(p.configPath, message) } func (p *Promoter) saveConfig() error { From 7efedf39fca57c965035552aa113299436af955d Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 10 Jun 2026 23:18:03 -0400 Subject: [PATCH 2/3] feat(hotfix): add finalize verb writing diverged state, tag, and release After a hotfix resolution PR merges and its build and deploy succeed, finalize cross-checks the merge SHA against the env branch tip, allocates the next free hotfix version (nested rc hotfix segment for rc-based versions, reconciled patch bump for published bases), snapshots prior state into the rollback ring, writes the divergence fields and build/deploy substates, commits the manifest to trunk, and creates the hotfix tag and release object. Prerelease-env targets promote the release to a prerelease. The verb is idempotent on identical inputs. Adds regression tests proving RC tag and draft cleanup ignore hotfix tags. Signed-off-by: Joshua Temple --- internal/hotfix/command.go | 92 ++++ internal/hotfix/finalize.go | 440 +++++++++++++++++ internal/hotfix/finalize_integration_test.go | 120 +++++ internal/hotfix/finalize_test.go | 472 +++++++++++++++++++ internal/release/hotfix_regression_test.go | 121 +++++ 5 files changed, 1245 insertions(+) create mode 100644 internal/hotfix/finalize.go create mode 100644 internal/hotfix/finalize_integration_test.go create mode 100644 internal/hotfix/finalize_test.go create mode 100644 internal/release/hotfix_regression_test.go diff --git a/internal/hotfix/command.go b/internal/hotfix/command.go index 1a23d197..9b05b18c 100644 --- a/internal/hotfix/command.go +++ b/internal/hotfix/command.go @@ -29,9 +29,101 @@ state write run in the generated workflow.`, } cmd.AddCommand(newPlanCommand()) + cmd.AddCommand(newFinalizeCommand()) return cmd } +// newFinalizeCommand creates the `cascade hotfix finalize` subcommand. +func newFinalizeCommand() *cobra.Command { + var ( + configPath string + manifestKey string + targetEnv string + mergeSHA string + fixSHA string + baseSHA string + actor string + repo string + dryRun bool + deployFlags []string + buildFlags []string + ) + + cmd := &cobra.Command{ + Use: "finalize", + Short: "Write diverged state, tag, and release for a merged hotfix", + Long: `Finalize a completed hotfix. + +After the resolution PR merges and the build and deploy succeed, this command: + 1. Cross-checks the merge SHA equals the env/ branch tip + 2. Allocates the next free hotfix version over the env's current version + 3. Snapshots the prior env state into the rollback ring + 4. Writes the diverged state (sha, version, ref, base_sha, patches) and substates + 5. Commits the manifest to trunk with the rebase-retry push + 6. Creates the hotfix tag and release object + +The verb is idempotent on identical inputs: a rerun after the state already +records the merge SHA is a no-op.`, + RunE: func(cmd *cobra.Command, args []string) error { + opts := []FinalizeOption{WithFinalizeDryRun(dryRun)} + + finalizer, err := NewFinalizer(FinalizerOptions{ + ConfigPath: configPath, + ManifestKey: manifestKey, + Actor: actor, + }, opts...) + if err != nil { + return err + } + + for _, df := range deployFlags { + name, result, ok := splitResultFlag(df) + if !ok { + return fmt.Errorf("invalid --deploy-result %q: want name=result", df) + } + finalizer.SetDeployResult(name, result) + } + for _, bf := range buildFlags { + name, result, ok := splitResultFlag(bf) + if !ok { + return fmt.Errorf("invalid --build-result %q: want name=result", bf) + } + finalizer.SetBuildResult(name, result) + } + + return finalizer.Finalize(targetEnv, mergeSHA, fixSHA, baseSHA) + }, + } + + cmd.Flags().StringVarP(&configPath, "config", "c", "", "Path to manifest file (default: .github/manifest.yaml)") + cmd.Flags().StringVar(&manifestKey, "key", config.DefaultManifestKey, "Top-level manifest key") + cmd.Flags().StringVar(&targetEnv, "target-env", "", "Environment to finalize (required)") + cmd.Flags().StringVar(&mergeSHA, "merge-sha", "", "Tip of env/ after the resolution PR merged (required)") + cmd.Flags().StringVar(&fixSHA, "fix-sha", "", "Trunk commit the hotfix carries (required)") + cmd.Flags().StringVar(&baseSHA, "base-sha", "", "Trunk anchor the integration branch diverged from (required)") + cmd.Flags().StringVar(&actor, "actor", "", "Actor recorded on the state (default: $GITHUB_ACTOR)") + cmd.Flags().StringVar(&repo, "repo", "", "owner/repo (unused placeholder for parity with plan)") + cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Validate and compute without writing state, tags, or releases") + cmd.Flags().StringArrayVar(&deployFlags, "deploy-result", nil, "Deploy result as name=result (repeatable)") + cmd.Flags().StringArrayVar(&buildFlags, "build-result", nil, "Build result as name=result (repeatable)") + + _ = cmd.MarkFlagRequired("target-env") + _ = cmd.MarkFlagRequired("merge-sha") + _ = cmd.MarkFlagRequired("fix-sha") + _ = cmd.MarkFlagRequired("base-sha") + + return cmd +} + +// splitResultFlag parses a "name=result" job-result flag value. +func splitResultFlag(s string) (name, result string, ok bool) { + idx := strings.IndexByte(s, '=') + if idx <= 0 || idx == len(s)-1 { + return "", "", false + } + return s[:idx], s[idx+1:], true +} + // newPlanCommand creates the `cascade hotfix plan` subcommand. func newPlanCommand() *cobra.Command { var ( diff --git a/internal/hotfix/finalize.go b/internal/hotfix/finalize.go new file mode 100644 index 00000000..88236219 --- /dev/null +++ b/internal/hotfix/finalize.go @@ -0,0 +1,440 @@ +package hotfix + +import ( + "fmt" + "os" + "time" + + "gopkg.in/yaml.v3" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stablekernel/cascade/internal/git" + "github.com/stablekernel/cascade/internal/release" + "github.com/stablekernel/cascade/internal/version" +) + +// releaseManager is the subset of release operations the finalize verb needs. +// The production implementation is *release.Manager; tests inject a stub. It is +// a small interface with a single method so callers without GitHub context are +// not forced to provide one. +type releaseManager interface { + Manage(opts release.Options) (*release.Result, error) +} + +// tagLister returns the repository's tags so the finalize verb can allocate the +// next free hotfix version without colliding with existing tags. The default +// implementation lists local git tags; tests inject a fixed set. +type tagLister interface { + ListTags() ([]string, error) +} + +// statePusher commits the manifest change to trunk and pushes it with the +// rebase-retry behavior promote uses. The default implementation reuses the +// shared git helper; tests inject a recorder. +type statePusher interface { + CommitAndPush(path, message string) error +} + +// gitTipReader resolves the tip SHA of a local branch. The default implementation +// shells out to git; tests reuse the planner's execGitRunner. +type gitTipReader interface { + LocalBranchSHA(name string) (string, error) +} + +// execTagLister lists local git tags. +type execTagLister struct{} + +func (execTagLister) ListTags() ([]string, error) { + tags, err := git.ListTags() + if err != nil { + return nil, fmt.Errorf("listing tags: %w", err) + } + return tags, nil +} + +// gitStatePusher commits and pushes the manifest with the promote rebase-retry. +type gitStatePusher struct{} + +func (gitStatePusher) CommitAndPush(path, message string) error { + return git.CommitAndPushWithRetry(path, message) +} + +// Finalizer writes the diverged state, tag, and release object for a completed +// hotfix. It mirrors the inputs of promote's Finalizer but targets one env on +// its integration branch rather than a trunk promotion. +type Finalizer struct { + cicd *config.CICDFile + configPath string + manifestKey string + actor string + dryRun bool + + deployResults map[string]string + buildResults map[string]string + + releaseMgr releaseManager + tagLister tagLister + pusher statePusher + tipReader gitTipReader +} + +// FinalizerOptions carries the required inputs for NewFinalizer. +type FinalizerOptions struct { + ConfigPath string + ManifestKey string + Actor string +} + +// FinalizeOption configures optional, additive Finalizer behavior. +type FinalizeOption func(*Finalizer) + +// WithFinalizeDryRun computes the finalize plan without writing state, tags, or +// release objects. +func WithFinalizeDryRun(dryRun bool) FinalizeOption { + return func(f *Finalizer) { f.dryRun = dryRun } +} + +// WithReleaseManager injects the release operations. When unset, finalize builds +// a *release.Manager from GITHUB_REPOSITORY and the release token at run time. +func WithReleaseManager(m releaseManager) FinalizeOption { + return func(f *Finalizer) { + if m != nil { + f.releaseMgr = m + } + } +} + +// WithTagLister injects the existing-tag lookup used for version allocation. +func WithTagLister(l tagLister) FinalizeOption { + return func(f *Finalizer) { + if l != nil { + f.tagLister = l + } + } +} + +// WithStatePusher injects the manifest commit/push. The default reuses the +// shared rebase-retry helper. +func WithStatePusher(p statePusher) FinalizeOption { + return func(f *Finalizer) { + if p != nil { + f.pusher = p + } + } +} + +// NewFinalizer constructs a Finalizer over the manifest at opts.ConfigPath. +func NewFinalizer(opts FinalizerOptions, options ...FinalizeOption) (*Finalizer, error) { + key := opts.ManifestKey + if key == "" { + key = config.DefaultManifestKey + } + + cicd, err := config.ParseManifestFile(opts.ConfigPath, key) + if err != nil { + return nil, fmt.Errorf("failed to parse config: %w", err) + } + + actor := opts.Actor + if actor == "" { + if a := os.Getenv("GITHUB_ACTOR"); a != "" { + actor = a + } else { + actor = "github-actions[bot]" + } + } + + f := &Finalizer{ + cicd: cicd, + configPath: opts.ConfigPath, + manifestKey: key, + actor: actor, + deployResults: make(map[string]string), + buildResults: make(map[string]string), + tagLister: execTagLister{}, + pusher: gitStatePusher{}, + tipReader: execGitRunner{}, + } + for _, o := range options { + o(f) + } + return f, nil +} + +// SetDeployResult records the result of a deploy job, mirroring +// promote.Finalizer.SetDeployResult. Valid results: "success", "failure", +// "skipped", "cancelled". Only successful deploys update per-deploy state. +func (f *Finalizer) SetDeployResult(name, result string) { + f.deployResults[name] = result +} + +// SetBuildResult records the conclusion of a build job. Valid results mirror +// SetDeployResult; only successful builds update per-build state. +func (f *Finalizer) SetBuildResult(name, result string) { + f.buildResults[name] = result +} + +// Finalize writes the diverged state for a completed hotfix on env/. +// +// targetEnv is the environment being hotfixed; mergeSHA is the tip of +// env/ after the resolution PR merged; fixSHA is the trunk commit the +// hotfix carries; baseSHA is the trunk anchor the integration branch diverged +// from. It cross-checks the merge SHA against the env-branch tip, allocates the +// next free hotfix version, snapshots the prior state into the Previous ring, +// writes the divergence fields and substates, commits the manifest to trunk, and +// creates the hotfix tag and release object. +// +// Finalize is idempotent on identical inputs: a rerun after the state already +// records the merge SHA is a no-op that neither double-applies patches nor +// re-snapshots Previous. +func (f *Finalizer) Finalize(targetEnv, mergeSHA, fixSHA, baseSHA string) error { + cfg := f.cicd.Config + if cfg == nil { + return fmt.Errorf("manifest has no config block") + } + if cfg.GetEnvironmentIndex(targetEnv) == -1 { + return fmt.Errorf("%q is not a configured environment", targetEnv) + } + + prior := f.cicd.State[targetEnv] + if prior == nil || prior.SHA == "" { + return fmt.Errorf("environment %q has no recorded state SHA", targetEnv) + } + + branch := envBranch(targetEnv) + + // Idempotency gate: if state already records the merge SHA, finalize already + // ran for these inputs. Re-running must not double-apply. + if prior.SHA == mergeSHA { + return nil + } + + // Cross-check the merge SHA equals the env-branch tip. + tip, err := f.tipReader.LocalBranchSHA(branch) + if err != nil { + return fmt.Errorf("reading tip of %s: %w", branch, err) + } + if tip != mergeSHA { + return fmt.Errorf( + "merge SHA %s does not match %s tip %s; the resolution branch advanced or an earlier run was interrupted: re-run finalize for the actual tip", + short(mergeSHA), branch, short(tip)) + } + + // Allocate the next free hotfix version over the prior version. + hotfixVersion, err := f.allocateVersion(prior.Version) + if err != nil { + return err + } + + timestamp := time.Now().UTC().Format(time.RFC3339) + + if f.dryRun { + return nil + } + + // Snapshot the prior state into the Previous ring (newest first). + snapshot := config.EnvStateSnapshot{ + SHA: prior.SHA, + Version: prior.Version, + CommittedAt: prior.CommittedAt, + CommittedBy: prior.CommittedBy, + } + prior.Previous = append([]config.EnvStateSnapshot{snapshot}, prior.Previous...) + + // Carry BaseSHA forward when already diverged; otherwise anchor it now. + if prior.BaseSHA == "" { + prior.BaseSHA = baseSHA + } + prior.Patches = append(prior.Patches, fixSHA) + prior.Ref = branch + prior.SHA = mergeSHA + prior.Version = hotfixVersion + prior.CommittedAt = timestamp + prior.CommittedBy = f.actor + + // Record per-deploy and per-build substates for successful jobs. + f.recordSubstates(prior, mergeSHA, hotfixVersion, timestamp) + + if err := f.writeConfig(); err != nil { + return err + } + + message := fmt.Sprintf("chore: record hotfix %s on %s [skip ci]", hotfixVersion, targetEnv) + if err := f.pusher.CommitAndPush(f.configPath, message); err != nil { + return fmt.Errorf("committing hotfix state: %w", err) + } + + // Create the hotfix tag and release object. + if err := f.createRelease(cfg, targetEnv, mergeSHA, hotfixVersion, fixSHA, prior.Version); err != nil { + return err + } + + return nil +} + +// allocateVersion returns the next free hotfix version over priorVersion. +// +// For an rc-based version (e.g. v1.4.0-rc.2) it allocates the next free dotted +// vX.Y.Z-rc.N.hotfix.M, skipping any hotfix tag that already exists. For a +// published version (e.g. v1.3.0, no rc segment) it allocates the next free +// patch bump (v1.3.1, v1.3.2, ...), reconciling against existing tags so it does +// not collide with a patch the normal release flow may also mint. +func (f *Finalizer) allocateVersion(priorVersion string) (string, error) { + if priorVersion == "" { + return "", fmt.Errorf("target environment has no recorded version; cannot allocate a hotfix version") + } + v, err := version.Parse(priorVersion) + if err != nil { + return "", fmt.Errorf("parsing target version %q: %w", priorVersion, err) + } + + tags, err := f.tagLister.ListTags() + if err != nil { + return "", err + } + existing := make(map[string]bool, len(tags)) + for _, t := range tags { + existing[t] = true + } + + if v.PreRelease >= 0 { + // RC-based: nested .hotfix.M segment. + for m := 1; ; m++ { + candidate := v.WithHotfix(m).String() + if !existing[candidate] { + return candidate, nil + } + } + } + + // Published base: normal patch bump, reconciled against existing tags. + next := v + for { + next = next.Bump(version.BumpPatch) + candidate := next.String() + if !existing[candidate] { + return candidate, nil + } + } +} + +// recordSubstates writes per-deploy and per-build substates for successful jobs, +// mirroring promote's finalize substate handling. +func (f *Finalizer) recordSubstates(state *config.EnvState, sha, ver, timestamp string) { + for name, result := range f.deployResults { + if result != "success" { + continue + } + if state.Deploys == nil { + state.Deploys = make(map[string]*config.DeployState) + } + if state.Deploys[name] == nil { + state.Deploys[name] = &config.DeployState{} + } + ds := state.Deploys[name] + ds.SHA = sha + ds.Version = ver + ds.DeployedAt = timestamp + ds.DeployedBy = f.actor + } + + for name, result := range f.buildResults { + if result != "success" { + continue + } + if state.Builds == nil { + state.Builds = make(map[string]*config.BuildState) + } + if state.Builds[name] == nil { + state.Builds[name] = &config.BuildState{} + } + bs := state.Builds[name] + bs.SHA = sha + bs.BuiltAt = timestamp + bs.BuiltBy = f.actor + } +} + +// createRelease creates the hotfix tag and release object. For a prerelease-env +// target the release is promoted to a GitHub prerelease, superseding the env's +// current prerelease object; for other envs it stays a draft. +func (f *Finalizer) createRelease(cfg *config.TrunkConfig, targetEnv, sha, hotfixVersion, fixSHA, baseVersion string) error { + mgr, err := f.resolveReleaseManager() + if err != nil { + return err + } + + body := fmt.Sprintf("Hotfix based on %s, carries trunk commit %s.", baseVersion, short(fixSHA)) + + if _, err := mgr.Manage(release.Options{ + Action: release.ActionCreate, + Environment: targetEnv, + SHA: sha, + Tag: hotfixVersion, + Changelog: body, + CreateTag: true, + }); err != nil { + return fmt.Errorf("creating hotfix release: %w", err) + } + + if f.isPrereleaseEnv(cfg, targetEnv) { + if _, err := mgr.Manage(release.Options{ + Action: release.ActionPrerelease, + Environment: targetEnv, + SHA: sha, + Tag: hotfixVersion, + }); err != nil { + return fmt.Errorf("promoting hotfix release to prerelease: %w", err) + } + } + + return nil +} + +// resolveReleaseManager returns the injected release manager or builds one from +// the environment. +func (f *Finalizer) resolveReleaseManager() (releaseManager, error) { + if f.releaseMgr != nil { + return f.releaseMgr, nil + } + repo := os.Getenv("GITHUB_REPOSITORY") + if repo == "" { + return nil, fmt.Errorf("GITHUB_REPOSITORY is not set; cannot create the hotfix release") + } + return release.NewManager(repo, releaseToken()), nil +} + +// releaseToken resolves the GitHub token for release operations from the +// environment, preferring an explicit RELEASE_TOKEN. +func releaseToken() string { + if t := os.Getenv("RELEASE_TOKEN"); t != "" { + return t + } + return os.Getenv("GITHUB_TOKEN") +} + +// isPrereleaseEnv reports whether env is the prerelease env (second from top), +// mirroring promote's prerelease-env detection. +func (f *Finalizer) isPrereleaseEnv(cfg *config.TrunkConfig, env string) bool { + envs := cfg.Environments + if len(envs) < 2 { + return false + } + return env == envs[len(envs)-2] +} + +// writeConfig writes the updated manifest back to disk, wrapped in the manifest +// key, matching the layout promote's finalize produces. +func (f *Finalizer) writeConfig() error { + wrapper := map[string]any{ + f.manifestKey: f.cicd, + } + data, err := yaml.Marshal(wrapper) + if err != nil { + return fmt.Errorf("marshaling manifest: %w", err) + } + if err := os.WriteFile(f.configPath, data, 0o600); err != nil { + return fmt.Errorf("writing manifest: %w", err) + } + return nil +} diff --git a/internal/hotfix/finalize_integration_test.go b/internal/hotfix/finalize_integration_test.go new file mode 100644 index 00000000..8b3cde9a --- /dev/null +++ b/internal/hotfix/finalize_integration_test.go @@ -0,0 +1,120 @@ +package hotfix + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stablekernel/cascade/internal/release" +) + +// TestFinalize_Integration_PlanThenMergeThenFinalize walks the full hotfix +// lifecycle against a real temporary git repository and a release-stub server: +// plan reconciles env/ at the recorded base, a manual cherry-pick plus +// merge advances the branch tip, and finalize writes the diverged manifest, +// allocates the hotfix version, and drives the release API. This is the +// committed scratch-repo plus release-stub coverage that complements the focused +// unit tests; full act/gitea e2e for the hotfix flow (plan, apply, finalize) +// lands in the e2e harness unit per the implementation plan. +func TestFinalize_Integration_PlanThenMergeThenFinalize(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "fix.txt", "patched", "fix on trunk") + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + // Step 1: plan reconciles env/test at the recorded base SHA. + planner := newPlanner(t, manifest) + planRes, err := planner.Plan(fix, "test") + if err != nil { + t.Fatalf("plan: %v", err) + } + if planRes.NoOp { + t.Fatal("expected a real hotfix, got no-op") + } + if got := gitOut(t, "rev-parse", "env/test"); got != base { + t.Fatalf("env/test tip = %q, want recorded base %q", got, base) + } + + // Step 2: simulate the resolution: cherry-pick the fix onto env/test and + // "merge" it (the branch tip is what the workflow would record as merge SHA). + runGit(t, "checkout", "env/test") + runGit(t, "cherry-pick", "-x", fix) + mergeSHA := gitOut(t, "rev-parse", "env/test") + runGit(t, "checkout", "main") + + // Step 3: finalize against a release-stub server. + var createBody string + var sawCreate bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/releases"): + // cleanupStaleDrafts list: no drafts. + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode([]release.GitHubRelease{}) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/git/refs"): + // createGitTag. + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{}) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/releases"): + var payload map[string]any + _ = json.NewDecoder(r.Body).Decode(&payload) + if b, ok := payload["body"].(string); ok { + createBody = b + } + sawCreate = true + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(release.GitHubRelease{ID: 1}) + default: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{}) + } + })) + defer server.Close() + + mgr := release.NewManagerWithURL("owner/repo", "token", server.URL) + + f := newFinalizer(t, manifest, + WithReleaseManager(mgr), + WithTagLister(stubTagLister{}), + WithStatePusher(&recordingPusher{}), + ) + f.SetDeployResult("api", "success") + f.SetBuildResult("api", "success") + + if err := f.Finalize("test", mergeSHA, fix, base); err != nil { + t.Fatalf("finalize: %v", err) + } + + // Manifest now records the diverged state. + st := loadState(t, manifest, "test") + if st.SHA != mergeSHA { + t.Errorf("state.sha = %q, want merge SHA %q", st.SHA, mergeSHA) + } + if st.Version != "v1.4.0-rc.2.hotfix.1" { + t.Errorf("state.version = %q, want v1.4.0-rc.2.hotfix.1", st.Version) + } + if st.Ref != "env/test" || st.BaseSHA != base || len(st.Patches) != 1 { + t.Errorf("divergence fields wrong: ref=%q base=%q patches=%v", st.Ref, st.BaseSHA, st.Patches) + } + if st.Deploys["api"] == nil || st.Deploys["api"].SHA != mergeSHA { + t.Errorf("deploy substate not recorded for api: %+v", st.Deploys) + } + if st.Builds["api"] == nil || st.Builds["api"].SHA != mergeSHA { + t.Errorf("build substate not recorded for api: %+v", st.Builds) + } + + // The release object was created with the hotfix tag and a base-version body. + if !sawCreate { + t.Fatal("expected a release create call") + } + if !strings.Contains(createBody, "v1.4.0-rc.2") || !strings.Contains(createBody, short(fix)) { + t.Errorf("release body missing base version or carried commit: %q", createBody) + } +} diff --git a/internal/hotfix/finalize_test.go b/internal/hotfix/finalize_test.go new file mode 100644 index 00000000..f6ad7cd1 --- /dev/null +++ b/internal/hotfix/finalize_test.go @@ -0,0 +1,472 @@ +package hotfix + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stablekernel/cascade/internal/release" +) + +// stubReleaseManager records the release operations finalize performs so tests +// can assert on the tag, body, and action without a live GitHub API. +type stubReleaseManager struct { + calls []release.Options + err error +} + +func (s *stubReleaseManager) Manage(opts release.Options) (*release.Result, error) { + s.calls = append(s.calls, opts) + if s.err != nil { + return nil, s.err + } + return &release.Result{ReleaseID: int64(len(s.calls)), HTMLURL: "https://example.test/releases/" + opts.Tag}, nil +} + +// stubTagLister returns a fixed set of existing tags for version allocation. +type stubTagLister struct { + tags []string +} + +func (s stubTagLister) ListTags() ([]string, error) { return s.tags, nil } + +// recordingPusher records that the manifest commit/push happened and how many +// times, so idempotency tests can assert the state write occurs exactly once. +type recordingPusher struct { + calls int + messages []string +} + +func (r *recordingPusher) CommitAndPush(path, message string) error { + r.calls++ + r.messages = append(r.messages, message) + return nil +} + +type envFixture struct { + sha string + version string + ref string + baseSHA string + patches []string +} + +// writeFinalizeManifest writes a manifest with the given environments and a rich +// per-env state block, returning its path. +func writeFinalizeManifest(t *testing.T, envs []string, states map[string]envFixture) string { + t.Helper() + + var b strings.Builder + b.WriteString("ci:\n") + b.WriteString(" config:\n") + b.WriteString(" environments:\n") + for _, e := range envs { + b.WriteString(" - " + e + "\n") + } + b.WriteString(" state:\n") + for e, f := range states { + b.WriteString(" " + e + ":\n") + b.WriteString(" sha: " + f.sha + "\n") + b.WriteString(" version: " + f.version + "\n") + if f.ref != "" { + b.WriteString(" ref: " + f.ref + "\n") + b.WriteString(" base_sha: " + f.baseSHA + "\n") + if len(f.patches) > 0 { + b.WriteString(" patches:\n") + for _, p := range f.patches { + b.WriteString(" - " + p + "\n") + } + } + } + } + + path := filepath.Join(".", "manifest.yaml") + if err := os.WriteFile(path, []byte(b.String()), 0o600); err != nil { + t.Fatalf("write manifest: %v", err) + } + return path +} + +// newFinalizer builds a Finalizer over the manifest with the supplied stubs. +func newFinalizer(t *testing.T, manifest string, opts ...FinalizeOption) *Finalizer { + t.Helper() + f, err := NewFinalizer(FinalizerOptions{ConfigPath: manifest, ManifestKey: "ci", Actor: "tester"}, opts...) + if err != nil { + t.Fatalf("NewFinalizer: %v", err) + } + return f +} + +// loadState reparses the manifest from disk and returns the target env state. +func loadState(t *testing.T, manifest, env string) *config.EnvState { + t.Helper() + cicd, err := config.ParseManifestFile(manifest, "ci") + if err != nil { + t.Fatalf("reparse manifest: %v", err) + } + return cicd.State[env] +} + +func TestFinalize_WritesDivergedState(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix on trunk") + + // env/test exists at the merge SHA (cherry-pick of the fix onto base). For + // the unit we model the merge SHA as a real commit on a hotfix branch. + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + merge := commitFile(t, "c.txt", "fixed", "cherry-pick fix") + runGit(t, "checkout", "main") + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + rm := &stubReleaseManager{} + f := newFinalizer(t, manifest, + WithReleaseManager(rm), + WithTagLister(stubTagLister{}), + WithStatePusher(&recordingPusher{}), + ) + f.SetDeployResult("api", "success") + + if err := f.Finalize("test", merge, fix, base); err != nil { + t.Fatalf("Finalize: %v", err) + } + + st := loadState(t, manifest, "test") + if st.SHA != merge { + t.Errorf("state.sha = %q, want merge SHA %q", st.SHA, merge) + } + if st.Version != "v1.4.0-rc.2.hotfix.1" { + t.Errorf("state.version = %q, want v1.4.0-rc.2.hotfix.1", st.Version) + } + if st.Ref != "env/test" { + t.Errorf("state.ref = %q, want env/test", st.Ref) + } + if st.BaseSHA != base { + t.Errorf("state.base_sha = %q, want %q", st.BaseSHA, base) + } + if len(st.Patches) != 1 || st.Patches[0] != fix { + t.Errorf("state.patches = %v, want [%s]", st.Patches, fix) + } + if !st.IsDiverged() { + t.Error("finalized hotfix state should report IsDiverged") + } + if st.CommittedBy != "tester" { + t.Errorf("committed_by = %q, want tester", st.CommittedBy) + } + + // A hotfix tag/release was created for the merge SHA. + if len(rm.calls) == 0 { + t.Fatal("expected at least one release Manage call") + } + create := rm.calls[0] + if create.Action != release.ActionCreate { + t.Errorf("first action = %q, want create", create.Action) + } + if create.Tag != "v1.4.0-rc.2.hotfix.1" { + t.Errorf("release tag = %q, want v1.4.0-rc.2.hotfix.1", create.Tag) + } + if !create.CreateTag { + t.Error("hotfix release must create the git tag") + } + if create.SHA != merge { + t.Errorf("release SHA = %q, want merge SHA %q", create.SHA, merge) + } + if !strings.Contains(create.Changelog, "v1.4.0-rc.2") { + t.Errorf("release body should reference the base version: %q", create.Changelog) + } + if !strings.Contains(create.Changelog, short(fix)) { + t.Errorf("release body should reference the carried trunk commit: %q", create.Changelog) + } +} + +func TestFinalize_PreviousRingSnapshot(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + merge := commitFile(t, "c.txt", "fixed", "cp") + runGit(t, "checkout", "main") + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + f := newFinalizer(t, manifest, + WithReleaseManager(&stubReleaseManager{}), + WithTagLister(stubTagLister{}), + WithStatePusher(&recordingPusher{}), + ) + if err := f.Finalize("test", merge, fix, base); err != nil { + t.Fatalf("Finalize: %v", err) + } + + st := loadState(t, manifest, "test") + if len(st.Previous) != 1 { + t.Fatalf("expected exactly one Previous snapshot, got %d", len(st.Previous)) + } + prev := st.Previous[0] + if prev.SHA != base { + t.Errorf("snapshot sha = %q, want prior sha %q", prev.SHA, base) + } + if prev.Version != "v1.4.0-rc.2" { + t.Errorf("snapshot version = %q, want prior version v1.4.0-rc.2", prev.Version) + } +} + +func TestFinalize_StacksSecondHotfix(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix1 := commitFile(t, "b.txt", "two", "first fix") + fix2 := commitFile(t, "d.txt", "four", "second fix") + + // env/test already carries the first hotfix; its tip is merge1. The second + // hotfix stacks another commit (merge2) on top. + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + commitFile(t, "c.txt", "fixed", "cp first") + merge2 := commitFile(t, "e.txt", "fixed2", "cp second") + tip := gitOut(t, "rev-parse", "env/test") + runGit(t, "checkout", "main") + _ = merge2 + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix2, version: "v1.4.0-rc.2"}, + "test": { + sha: gitOut(t, "rev-parse", "env/test~1"), + version: "v1.4.0-rc.2.hotfix.1", + ref: "env/test", + baseSHA: base, + patches: []string{fix1}, + }, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + f := newFinalizer(t, manifest, + WithReleaseManager(&stubReleaseManager{}), + WithTagLister(stubTagLister{tags: []string{"v1.4.0-rc.2.hotfix.1"}}), + WithStatePusher(&recordingPusher{}), + ) + if err := f.Finalize("test", tip, fix2, base); err != nil { + t.Fatalf("Finalize: %v", err) + } + + st := loadState(t, manifest, "test") + if st.Version != "v1.4.0-rc.2.hotfix.2" { + t.Errorf("second hotfix version = %q, want v1.4.0-rc.2.hotfix.2", st.Version) + } + if st.BaseSHA != base { + t.Errorf("base_sha = %q, want carried-forward %q", st.BaseSHA, base) + } + if len(st.Patches) != 2 || st.Patches[0] != fix1 || st.Patches[1] != fix2 { + t.Errorf("patches = %v, want [%s %s]", st.Patches, fix1, fix2) + } +} + +func TestFinalize_MergeSHATipMismatch_Fails(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + commitFile(t, "c.txt", "fixed", "cp") + runGit(t, "checkout", "main") + // other is NOT the tip of env/test. + other := commitFile(t, "z.txt", "zee", "unrelated") + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + f := newFinalizer(t, manifest, + WithReleaseManager(&stubReleaseManager{}), + WithTagLister(stubTagLister{}), + WithStatePusher(&recordingPusher{}), + ) + err := f.Finalize("test", other, fix, base) + if err == nil { + t.Fatal("expected mismatch error when merge SHA is not env/test tip") + } + if !strings.Contains(strings.ToLower(err.Error()), "tip") { + t.Errorf("error %q should mention the branch tip mismatch", err.Error()) + } +} + +func TestFinalize_Idempotent_Rerun(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + merge := commitFile(t, "c.txt", "fixed", "cp") + runGit(t, "checkout", "main") + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + pusher := &recordingPusher{} + rm := &stubReleaseManager{} + + // First run. + f1 := newFinalizer(t, manifest, + WithReleaseManager(rm), + WithTagLister(stubTagLister{}), + WithStatePusher(pusher), + ) + if err := f1.Finalize("test", merge, fix, base); err != nil { + t.Fatalf("first Finalize: %v", err) + } + + st1 := loadState(t, manifest, "test") + if len(st1.Patches) != 1 { + t.Fatalf("after first run patches = %v, want one", st1.Patches) + } + + // Second run with identical inputs; the tag now exists. + f2 := newFinalizer(t, manifest, + WithReleaseManager(rm), + WithTagLister(stubTagLister{tags: []string{"v1.4.0-rc.2.hotfix.1"}}), + WithStatePusher(pusher), + ) + if err := f2.Finalize("test", merge, fix, base); err != nil { + t.Fatalf("second Finalize (idempotent): %v", err) + } + + st2 := loadState(t, manifest, "test") + if len(st2.Patches) != 1 { + t.Errorf("rerun double-applied patches: %v", st2.Patches) + } + if st2.Version != "v1.4.0-rc.2.hotfix.1" { + t.Errorf("rerun changed version to %q, want stable v1.4.0-rc.2.hotfix.1", st2.Version) + } + if len(st2.Previous) != 1 { + t.Errorf("rerun double-snapshotted Previous: %d entries", len(st2.Previous)) + } +} + +func TestFinalize_VersionAllocation_SkipsExistingTags(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + merge := commitFile(t, "c.txt", "fixed", "cp") + runGit(t, "checkout", "main") + + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + // hotfix.1 and hotfix.2 tags already exist; allocation must skip to hotfix.3. + rm := &stubReleaseManager{} + f := newFinalizer(t, manifest, + WithReleaseManager(rm), + WithTagLister(stubTagLister{tags: []string{"v1.4.0-rc.2.hotfix.1", "v1.4.0-rc.2.hotfix.2"}}), + WithStatePusher(&recordingPusher{}), + ) + if err := f.Finalize("test", merge, fix, base); err != nil { + t.Fatalf("Finalize: %v", err) + } + + st := loadState(t, manifest, "test") + if st.Version != "v1.4.0-rc.2.hotfix.3" { + t.Errorf("version = %q, want v1.4.0-rc.2.hotfix.3 (skipping existing tags)", st.Version) + } + if rm.calls[0].Tag != "v1.4.0-rc.2.hotfix.3" { + t.Errorf("release tag = %q, want v1.4.0-rc.2.hotfix.3", rm.calls[0].Tag) + } +} + +func TestFinalize_PublishedBase_PatchBump_NoCollision(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + runGit(t, "branch", "env/test", base) + runGit(t, "checkout", "env/test") + merge := commitFile(t, "c.txt", "fixed", "cp") + runGit(t, "checkout", "main") + + // test holds a PUBLISHED version v1.3.0 (no rc segment). + manifest := writeFinalizeManifest(t, []string{"dev", "test", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.1"}, + "test": {sha: base, version: "v1.3.0"}, + "prod": {sha: base, version: "v1.3.0"}, + }) + + // v1.3.1 already exists as a tag (e.g. the normal release flow minted it); + // allocation must skip it and choose v1.3.2 to avoid a collision. + rm := &stubReleaseManager{} + f := newFinalizer(t, manifest, + WithReleaseManager(rm), + WithTagLister(stubTagLister{tags: []string{"v1.3.0", "v1.3.1"}}), + WithStatePusher(&recordingPusher{}), + ) + if err := f.Finalize("test", merge, fix, base); err != nil { + t.Fatalf("Finalize: %v", err) + } + + st := loadState(t, manifest, "test") + if st.Version != "v1.3.2" { + t.Errorf("published-base hotfix version = %q, want v1.3.2 (patch bump skipping existing v1.3.1)", st.Version) + } + if strings.Contains(st.Version, "hotfix") { + t.Errorf("published-base hotfix must NOT use a -hotfix.M segment: %q", st.Version) + } +} + +func TestFinalize_PrereleaseEnv_ReplacesPrerelease(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + runGit(t, "branch", "env/uat", base) + runGit(t, "checkout", "env/uat") + merge := commitFile(t, "c.txt", "fixed", "cp") + runGit(t, "checkout", "main") + + // uat is the prerelease env (second from top). A hotfix there must promote + // the release object to a GitHub prerelease, replacing the env's current one. + manifest := writeFinalizeManifest(t, []string{"dev", "test", "uat", "prod"}, map[string]envFixture{ + "dev": {sha: fix, version: "v1.4.0-rc.2"}, + "test": {sha: fix, version: "v1.4.0-rc.2"}, + "uat": {sha: base, version: "v1.4.0-rc.2"}, + "prod": {sha: base, version: "v1.4.0-rc.2"}, + }) + + rm := &stubReleaseManager{} + f := newFinalizer(t, manifest, + WithReleaseManager(rm), + WithTagLister(stubTagLister{}), + WithStatePusher(&recordingPusher{}), + ) + if err := f.Finalize("uat", merge, fix, base); err != nil { + t.Fatalf("Finalize: %v", err) + } + + // The release flow must reach a prerelease action for the hotfix tag. + var sawPrerelease bool + for _, c := range rm.calls { + if c.Action == release.ActionPrerelease { + sawPrerelease = true + } + } + if !sawPrerelease { + t.Errorf("prerelease-env hotfix should promote the release to a prerelease; calls=%+v", rm.calls) + } +} diff --git a/internal/release/hotfix_regression_test.go b/internal/release/hotfix_regression_test.go new file mode 100644 index 00000000..67e5966a --- /dev/null +++ b/internal/release/hotfix_regression_test.go @@ -0,0 +1,121 @@ +package release + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The RC-shaped cleanup logic is intentionally blind to hotfix tags: hotfix +// versions use a nested .hotfix.M segment (or a plain patch bump) that the RC +// regexes do not match. These regression tests pin that contract so a future +// regex change cannot silently start deleting hotfix tags or drafts. + +func TestParseRCTag_IgnoresHotfixTags(t *testing.T) { + hotfixTags := []string{ + "v1.4.0-rc.2.hotfix.1", + "v1.4.0-rc.2.hotfix.2", + "v1.3.0-rc.10.hotfix.5", + } + for _, tag := range hotfixTags { + t.Run(tag, func(t *testing.T) { + base, rc, ok := parseRCTag(tag) + assert.False(t, ok, "hotfix tag must not parse as an RC tag") + assert.Equal(t, "", base) + assert.Equal(t, -1, rc) + + assert.False(t, isRCTag(tag), "hotfix tag must not be classified as an RC tag") + }) + } +} + +func TestCleanupRCTags_SkipsHotfixTags(t *testing.T) { + var deleted []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/repos/owner/repo/git/refs/tags": + refs := []map[string]string{ + {"ref": "refs/tags/v1.4.0-rc.0"}, + {"ref": "refs/tags/v1.4.0-rc.1"}, + {"ref": "refs/tags/v1.4.0-rc.1.hotfix.1"}, // must be preserved + {"ref": "refs/tags/v1.4.0-rc.1.hotfix.2"}, // must be preserved + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(refs) + case r.Method == http.MethodDelete: + // /repos/owner/repo/git/refs/tags/ + deleted = append(deleted, r.URL.Path) + w.WriteHeader(http.StatusNoContent) + default: + w.WriteHeader(http.StatusOK) + } + })) + defer server.Close() + + m := &Manager{client: server.Client(), baseURL: server.URL, token: "t", repo: "owner/repo"} + + err := m.cleanupRCTags("v1.4.0") + require.NoError(t, err) + + // Only the two plain RC tags are deleted; both hotfix tags are preserved. + assert.Len(t, deleted, 2) + for _, p := range deleted { + assert.NotContains(t, p, "hotfix", "cleanupRCTags must never delete a hotfix tag") + } +} + +func TestCleanupStaleDrafts_IgnoresHotfixDrafts(t *testing.T) { + var deletedIDs []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/repos/owner/repo/releases": + releases := []GitHubRelease{ + {ID: 1, TagName: "v1.4.0-rc.0", Name: "v1.4.0-rc.0", Draft: true}, + {ID: 2, TagName: "v1.4.0-rc.1", Name: "v1.4.0-rc.1", Draft: true}, + // A hotfix draft for the same base must be preserved. + {ID: 3, TagName: "v1.4.0-rc.1.hotfix.1", Name: "v1.4.0-rc.1.hotfix.1", Draft: true}, + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(releases) + case r.Method == http.MethodDelete: + deletedIDs = append(deletedIDs, r.URL.Path) + w.WriteHeader(http.StatusNoContent) + default: + w.WriteHeader(http.StatusOK) + } + })) + defer server.Close() + + m := &Manager{client: server.Client(), baseURL: server.URL, token: "t", repo: "owner/repo"} + + // Creating rc.2 cleans up rc.0 and rc.1 drafts but must leave the hotfix draft. + err := m.cleanupStaleDrafts("test", "v1.4.0-rc.2") + require.NoError(t, err) + + for _, p := range deletedIDs { + assert.NotContains(t, p, "/releases/3", "the hotfix draft (id 3) must be preserved") + } +} + +// TestCleanupStaleDrafts_HotfixCurrentTagIsNoOp confirms that when the current +// tag is itself a hotfix tag, no draft cleanup runs at all (parseRCTag returns +// not-ok for the hotfix tag, so cleanup short-circuits). +func TestCleanupStaleDrafts_HotfixCurrentTagIsNoOp(t *testing.T) { + called := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode([]GitHubRelease{}) + })) + defer server.Close() + + m := &Manager{client: server.Client(), baseURL: server.URL, token: "t", repo: "owner/repo"} + + err := m.cleanupStaleDrafts("test", "v1.4.0-rc.2.hotfix.1") + require.NoError(t, err) + assert.False(t, called, "a hotfix current tag must short-circuit before listing drafts") +} From ce498879fd9fd3df46c615053bc175e0196d614d Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 10 Jun 2026 23:26:06 -0400 Subject: [PATCH 3/3] fix: capture base version before state mutation in hotfix finalize Prior to this change, createRelease was passed prior.Version after it had already been overwritten with the newly allocated hotfix version, causing the release body to read "based on " rather than the true pre-hotfix base version. Capture baseVersion before the mutation and pass that instead. Tighten the release-body test to assert the exact "based on v1.4.0-rc.2," phrase and assert the hotfix version does not appear in the based-on position, so the test would have caught the original bug. Remove the dead --repo flag from the finalize subcommand; it was wired but never consumed and was documented as an unused placeholder. Signed-off-by: Joshua Temple --- internal/hotfix/command.go | 2 -- internal/hotfix/finalize.go | 5 ++++- internal/hotfix/finalize_test.go | 7 +++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/hotfix/command.go b/internal/hotfix/command.go index 9b05b18c..a8d777b3 100644 --- a/internal/hotfix/command.go +++ b/internal/hotfix/command.go @@ -43,7 +43,6 @@ func newFinalizeCommand() *cobra.Command { fixSHA string baseSHA string actor string - repo string dryRun bool deployFlags []string buildFlags []string @@ -102,7 +101,6 @@ records the merge SHA is a no-op.`, cmd.Flags().StringVar(&fixSHA, "fix-sha", "", "Trunk commit the hotfix carries (required)") cmd.Flags().StringVar(&baseSHA, "base-sha", "", "Trunk anchor the integration branch diverged from (required)") cmd.Flags().StringVar(&actor, "actor", "", "Actor recorded on the state (default: $GITHUB_ACTOR)") - cmd.Flags().StringVar(&repo, "repo", "", "owner/repo (unused placeholder for parity with plan)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Validate and compute without writing state, tags, or releases") cmd.Flags().StringArrayVar(&deployFlags, "deploy-result", nil, "Deploy result as name=result (repeatable)") cmd.Flags().StringArrayVar(&buildFlags, "build-result", nil, "Build result as name=result (repeatable)") diff --git a/internal/hotfix/finalize.go b/internal/hotfix/finalize.go index 88236219..ec4a1364 100644 --- a/internal/hotfix/finalize.go +++ b/internal/hotfix/finalize.go @@ -220,6 +220,9 @@ func (f *Finalizer) Finalize(targetEnv, mergeSHA, fixSHA, baseSHA string) error short(mergeSHA), branch, short(tip)) } + // Capture the base version before prior.Version is overwritten below. + baseVersion := prior.Version + // Allocate the next free hotfix version over the prior version. hotfixVersion, err := f.allocateVersion(prior.Version) if err != nil { @@ -265,7 +268,7 @@ func (f *Finalizer) Finalize(targetEnv, mergeSHA, fixSHA, baseSHA string) error } // Create the hotfix tag and release object. - if err := f.createRelease(cfg, targetEnv, mergeSHA, hotfixVersion, fixSHA, prior.Version); err != nil { + if err := f.createRelease(cfg, targetEnv, mergeSHA, hotfixVersion, fixSHA, baseVersion); err != nil { return err } diff --git a/internal/hotfix/finalize_test.go b/internal/hotfix/finalize_test.go index f6ad7cd1..6f3d35fa 100644 --- a/internal/hotfix/finalize_test.go +++ b/internal/hotfix/finalize_test.go @@ -179,8 +179,11 @@ func TestFinalize_WritesDivergedState(t *testing.T) { if create.SHA != merge { t.Errorf("release SHA = %q, want merge SHA %q", create.SHA, merge) } - if !strings.Contains(create.Changelog, "v1.4.0-rc.2") { - t.Errorf("release body should reference the base version: %q", create.Changelog) + if !strings.Contains(create.Changelog, "based on v1.4.0-rc.2,") { + t.Errorf("release body should reference the base version with exact phrase: %q", create.Changelog) + } + if strings.Contains(create.Changelog, "based on v1.4.0-rc.2.hotfix.1") { + t.Errorf("release body must not use the hotfix version as the base version: %q", create.Changelog) } if !strings.Contains(create.Changelog, short(fix)) { t.Errorf("release body should reference the carried trunk commit: %q", create.Changelog)