From 3ca2efaf3d7fffd8c4fdcecbb4e2758f5f34290a Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Mon, 22 Jun 2026 12:04:12 -0400 Subject: [PATCH] feat: plan multi-commit hotfix elevation across the env chain Signed-off-by: Joshua Temple --- internal/hotfix/chain.go | 192 ++++++++++++++++++ internal/hotfix/chain_test.go | 355 ++++++++++++++++++++++++++++++++++ internal/hotfix/command.go | 87 ++++++++- internal/hotfix/plan.go | 31 ++- 4 files changed, 644 insertions(+), 21 deletions(-) create mode 100644 internal/hotfix/chain.go create mode 100644 internal/hotfix/chain_test.go diff --git a/internal/hotfix/chain.go b/internal/hotfix/chain.go new file mode 100644 index 0000000..95d997d --- /dev/null +++ b/internal/hotfix/chain.go @@ -0,0 +1,192 @@ +package hotfix + +import ( + "fmt" + "slices" + "strings" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stablekernel/cascade/internal/git" +) + +// PlanChainResult is the computed, validated plan for elevating a set of trunk +// commits across the bottom-up environment chain up to and including a target +// environment. It is the multi-commit, multi-env companion to PlanResult and is +// additive: the single-env Plan and PlanResult are unchanged. +type PlanChainResult struct { + // Envs are the per-environment plans in bottom-up chain order (the first + // environment is excluded; the target environment is the last entry). + Envs []EnvPlan `json:"envs"` +} + +// EnvPlan is the plan for one environment in the chain: the env branch, its +// recorded base SHA, and the ordered list of commits still to apply after +// per-(commit,env) idempotency skips. NoOp is true when every requested commit +// is already present in that environment. +type EnvPlan struct { + Env string `json:"env"` + Branch string `json:"branch"` + BaseSHA string `json:"base_sha"` + + // Commits are the fix SHAs still to cherry-pick into this environment, in + // the caller's requested ref order, after skipping commits already present. + Commits []string `json:"commits"` + + // NoOp is true when the whole requested set is already present in this env. + NoOp bool `json:"no_op"` + + // ConflictExpected hints whether a cherry-pick is likely to conflict. The + // plan verb does not run the cherry-pick, so this is best-effort and false + // by default; the workflow is authoritative. + ConflictExpected bool `json:"conflict_expected"` +} + +// parseCommitRefs splits a comma-delimited commit-ref input into an ordered +// slice. It trims surrounding whitespace on each entry, rejects empty input and +// any empty or duplicate entry, and preserves order. A single ref returns a +// one-element slice, keeping the single-commit call site back-compatible. +func parseCommitRefs(input string) ([]string, error) { + if strings.TrimSpace(input) == "" { + return nil, fmt.Errorf("no commit refs given") + } + + parts := strings.Split(input, ",") + refs := make([]string, 0, len(parts)) + seen := make(map[string]struct{}, len(parts)) + for _, raw := range parts { + ref := strings.TrimSpace(raw) + if ref == "" { + return nil, fmt.Errorf("empty commit ref in %q", input) + } + if _, dup := seen[ref]; dup { + return nil, fmt.Errorf("duplicate commit ref %q", ref) + } + seen[ref] = struct{}{} + refs = append(refs, ref) + } + return refs, nil +} + +// expandTargetEnvs returns the bottom-up environment sequence to elevate +// through, from the second environment up to and including targetEnv. The first +// environment is excluded (a fix reaches it by merging to trunk) and any +// environment above targetEnv is excluded. It rejects an unknown target and the +// first environment. +func expandTargetEnvs(cfg *config.TrunkConfig, targetEnv string) ([]string, error) { + idx := cfg.GetEnvironmentIndex(targetEnv) + if idx == -1 { + return nil, fmt.Errorf("%q is not a configured environment", targetEnv) + } + if cfg.IsFirstEnvironment(targetEnv) { + return nil, fmt.Errorf("%q is the first environment; a fix reaches it by merging to trunk, not by hotfix", targetEnv) + } + // Environments[1..idx] inclusive: skip the first env, stop at the target. + seq := make([]string, 0, idx) + seq = append(seq, cfg.Environments[1:idx+1]...) + return seq, nil +} + +// resolveTrunkAncestors resolves each ref to a full SHA and verifies it is an +// ancestor of trunk tip, preserving order. The first ref that fails to resolve +// or is not on trunk aborts with an error naming that ref, so a mixed set +// surfaces the offending commit. The returned slice is aligned with refs. +func (p *Planner) resolveTrunkAncestors(refs []string) ([]string, error) { + trunkSHA, err := p.gitRunner.ResolveSHA("HEAD") + if err != nil { + return nil, fmt.Errorf("resolving trunk tip: %w", err) + } + + shas := make([]string, 0, len(refs)) + for _, ref := range refs { + sha, err := p.gitRunner.ResolveSHA(ref) + if err != nil { + return nil, fmt.Errorf("resolving fix commit %q: %w", ref, err) + } + onTrunk, err := git.IsAncestor(sha, trunkSHA) + if err != nil { + return nil, fmt.Errorf("checking trunk ancestry of %q: %w", ref, err) + } + if !onTrunk { + return nil, fmt.Errorf("commit %s (%s) is not on trunk: a hotfix must apply a commit that is already an ancestor of trunk; merge it to trunk first", short(sha), ref) + } + shas = append(shas, sha) + } + return shas, nil +} + +// commitPresentInEnv reports whether fixSHA is already present in an environment +// whose recorded state SHA is baseSHA and whose applied-patch list is patches. A +// commit counts as present when it is an ancestor of the state SHA OR it is +// already recorded in the patch list. The patch-list check is what makes +// re-elevation idempotent across the diverged env branch. +func commitPresentInEnv(fixSHA, baseSHA string, patches []string) (bool, error) { + if slices.Contains(patches, fixSHA) { + return true, nil + } + already, err := git.IsAncestor(fixSHA, baseSHA) + if err != nil { + return false, err + } + return already, nil +} + +// PlanChain validates and computes the per-environment plan for elevating a set +// of trunk commits across the bottom-up environment chain up to and including +// targetEnv. Commits are kept in the caller's ref order; environments run +// bottom-up. Each ref must resolve and be a trunk ancestor; each (commit, env) +// pair is skipped when the commit is already present in that environment (an +// ancestor of its state SHA or already in its patch list). An environment whose +// whole requested set is already present is reported as a no-op. +// +// PlanChain is additive: it does not create branches or mutate state, and the +// single-env Plan and PlanResult are unchanged. +func (p *Planner) PlanChain(refs []string, targetEnv string) (*PlanChainResult, error) { + cfg := p.cicd.Config + if cfg == nil { + return nil, fmt.Errorf("manifest has no config block") + } + if len(refs) == 0 { + return nil, fmt.Errorf("no commit refs given") + } + + // Per-ref trunk-ancestry gate over the whole set, order preserved. + shas, err := p.resolveTrunkAncestors(refs) + if err != nil { + return nil, err + } + + // Bottom-up environment sequence (excludes the first env, stops at target). + envs, err := expandTargetEnvs(cfg, targetEnv) + if err != nil { + return nil, err + } + + result := &PlanChainResult{Envs: make([]EnvPlan, 0, len(envs))} + for _, env := range envs { + state := p.cicd.State[env] + if state == nil || state.SHA == "" { + return nil, fmt.Errorf("environment %q has no recorded state SHA", env) + } + baseSHA := state.SHA + + ep := EnvPlan{ + Env: env, + Branch: envBranch(env), + BaseSHA: baseSHA, + Commits: make([]string, 0, len(shas)), + } + for _, sha := range shas { + present, err := commitPresentInEnv(sha, baseSHA, state.Patches) + if err != nil { + return nil, fmt.Errorf("checking whether commit is already in %q: %w", env, err) + } + if present { + continue + } + ep.Commits = append(ep.Commits, sha) + } + ep.NoOp = len(ep.Commits) == 0 + result.Envs = append(result.Envs, ep) + } + return result, nil +} diff --git a/internal/hotfix/chain_test.go b/internal/hotfix/chain_test.go new file mode 100644 index 0000000..3ec0ac3 --- /dev/null +++ b/internal/hotfix/chain_test.go @@ -0,0 +1,355 @@ +package hotfix + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stablekernel/cascade/internal/config" +) + +// envSpec describes one environment row for writeManifestFull: its recorded +// state SHA, optional version override, and optional applied patch SHAs. +type envSpec struct { + sha string + version string + patches []string +} + +// writeManifestFull writes a manifest with per-env state including optional +// version overrides and applied patch lists, and returns its path. It mirrors +// writeManifest but exposes the patches field the idempotency checks consult. +func writeManifestFull(t *testing.T, envs []string, state map[string]envSpec) 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 := range envs { + spec, ok := state[e] + if !ok { + continue + } + version := spec.version + if version == "" { + version = "v1.0.0-rc.1" + } + b.WriteString(" " + e + ":\n") + b.WriteString(" sha: " + spec.sha + "\n") + b.WriteString(" version: " + version + "\n") + if len(spec.patches) > 0 { + b.WriteString(" patches:\n") + for _, p := range spec.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 +} + +// --- T1: parseCommitRefs --------------------------------------------------- + +func TestParseCommitRefs(t *testing.T) { + tests := []struct { + name string + input string + want []string + wantErr bool + }{ + {name: "single ref", input: "abc123", want: []string{"abc123"}}, + {name: "two refs", input: "abc,def", want: []string{"abc", "def"}}, + {name: "trims whitespace", input: " abc , def ", want: []string{"abc", "def"}}, + {name: "preserves order", input: "c,a,b", want: []string{"c", "a", "b"}}, + {name: "empty input", input: "", wantErr: true}, + {name: "whitespace only", input: " ", wantErr: true}, + {name: "empty entry", input: "abc,,def", wantErr: true}, + {name: "trailing comma", input: "abc,", wantErr: true}, + {name: "duplicate entry", input: "abc,def,abc", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseCommitRefs(tt.input) + if tt.wantErr { + if err == nil { + t.Fatalf("parseCommitRefs(%q) = %v, want error", tt.input, got) + } + return + } + if err != nil { + t.Fatalf("parseCommitRefs(%q): %v", tt.input, err) + } + if len(got) != len(tt.want) { + t.Fatalf("parseCommitRefs(%q) = %v, want %v", tt.input, got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("parseCommitRefs(%q)[%d] = %q, want %q", tt.input, i, got[i], tt.want[i]) + } + } + }) + } +} + +// --- T3: expandTargetEnvs -------------------------------------------------- + +func TestExpandTargetEnvs(t *testing.T) { + envs := []string{"dev", "test", "staging", "prod"} + + tests := []struct { + name string + targetEnv string + want []string + wantErr bool + }{ + {name: "staging stops at staging", targetEnv: "staging", want: []string{"test", "staging"}}, + {name: "prod includes whole chain above first", targetEnv: "prod", want: []string{"test", "staging", "prod"}}, + {name: "second env alone", targetEnv: "test", want: []string{"test"}}, + {name: "first env rejected", targetEnv: "dev", wantErr: true}, + {name: "unknown env rejected", targetEnv: "qa", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &config.TrunkConfig{Environments: envs} + got, err := expandTargetEnvs(cfg, tt.targetEnv) + if tt.wantErr { + if err == nil { + t.Fatalf("expandTargetEnvs(%q) = %v, want error", tt.targetEnv, got) + } + return + } + if err != nil { + t.Fatalf("expandTargetEnvs(%q): %v", tt.targetEnv, err) + } + if strings.Join(got, ",") != strings.Join(tt.want, ",") { + t.Errorf("expandTargetEnvs(%q) = %v, want %v", tt.targetEnv, got, tt.want) + } + }) + } +} + +// --- T4: per-(commit,env) idempotent skip ---------------------------------- + +func TestPlan_SkipsCommitAlreadyInPatches(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix on trunk") + + // test's recorded state SHA is base (does NOT contain fix as an ancestor), + // but the fix SHA is already listed in patches: this is the core gap. The + // old no-op check only consulted state.SHA ancestry and would have planned + // the cherry-pick again. + manifest := writeManifestFull(t, []string{"dev", "test", "prod"}, map[string]envSpec{ + "dev": {sha: fix}, + "test": {sha: base, patches: []string{fix}}, + "prod": {sha: base}, + }) + + p := newPlanner(t, manifest) + res, err := p.Plan(fix, "test") + if err != nil { + t.Fatalf("Plan: %v", err) + } + if !res.NoOp { + t.Errorf("expected NoOp=true when fix is already in state.patches, got %+v", res) + } +} + +func TestPlan_SkipsCommitAlreadyInStateSHA(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix := commitFile(t, "b.txt", "two", "fix") + tip := commitFile(t, "c.txt", "three", "later") + + // test points at tip, which already contains fix as an ancestor. + manifest := writeManifestFull(t, []string{"dev", "test", "prod"}, map[string]envSpec{ + "dev": {sha: base}, + "test": {sha: tip}, + "prod": {sha: base}, + }) + + p := newPlanner(t, manifest) + res, err := p.Plan(fix, "test") + if err != nil { + t.Fatalf("Plan: %v", err) + } + if !res.NoOp { + t.Errorf("expected NoOp=true when fix is an ancestor of state.SHA, got %+v", res) + } +} + +// --- T5: PlanChain orchestration ------------------------------------------- + +func TestPlanChain_CleanSet(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix1 := commitFile(t, "b.txt", "two", "fix one") + fix2 := commitFile(t, "c.txt", "three", "fix two") + + // envs: dev (first, excluded), test, staging, prod. Target staging => the + // chain is [test, staging]. Neither env contains the fixes, so both are + // planned with both commits in ref order. + manifest := writeManifestFull(t, []string{"dev", "test", "staging", "prod"}, map[string]envSpec{ + "dev": {sha: fix2}, + "test": {sha: base}, + "staging": {sha: base}, + "prod": {sha: base}, + }) + + p := newPlanner(t, manifest) + res, err := p.PlanChain([]string{fix1, fix2}, "staging") + if err != nil { + t.Fatalf("PlanChain: %v", err) + } + if len(res.Envs) != 2 { + t.Fatalf("expected 2 env plans, got %d: %+v", len(res.Envs), res.Envs) + } + if res.Envs[0].Env != "test" || res.Envs[1].Env != "staging" { + t.Errorf("env order = [%s,%s], want [test,staging]", res.Envs[0].Env, res.Envs[1].Env) + } + for _, ep := range res.Envs { + if ep.NoOp { + t.Errorf("env %s should not be a no-op", ep.Env) + } + if len(ep.Commits) != 2 { + t.Errorf("env %s commits = %v, want 2 in ref order", ep.Env, ep.Commits) + } + if len(ep.Commits) == 2 && (ep.Commits[0] != fix1 || ep.Commits[1] != fix2) { + t.Errorf("env %s commit order = %v, want [%s,%s]", ep.Env, ep.Commits, fix1, fix2) + } + if ep.Branch != "env/"+ep.Env { + t.Errorf("env %s branch = %q", ep.Env, ep.Branch) + } + if ep.BaseSHA != base { + t.Errorf("env %s base = %q, want %q", ep.Env, ep.BaseSHA, base) + } + } +} + +func TestPlanChain_FullyPresentEnvIsNoOp(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix1 := commitFile(t, "b.txt", "two", "fix one") + fix2 := commitFile(t, "c.txt", "three", "fix two") + + // test already carries both fixes via patches; staging carries neither. + manifest := writeManifestFull(t, []string{"dev", "test", "staging", "prod"}, map[string]envSpec{ + "dev": {sha: fix2}, + "test": {sha: base, patches: []string{fix1, fix2}}, + "staging": {sha: base}, + "prod": {sha: base}, + }) + + p := newPlanner(t, manifest) + res, err := p.PlanChain([]string{fix1, fix2}, "staging") + if err != nil { + t.Fatalf("PlanChain: %v", err) + } + if len(res.Envs) != 2 { + t.Fatalf("expected 2 env plans, got %d", len(res.Envs)) + } + test := res.Envs[0] + if test.Env != "test" || !test.NoOp || len(test.Commits) != 0 { + t.Errorf("test env should be a whole-set no-op, got %+v", test) + } + staging := res.Envs[1] + if staging.Env != "staging" || staging.NoOp || len(staging.Commits) != 2 { + t.Errorf("staging env should plan both commits, got %+v", staging) + } +} + +func TestPlanChain_MixedSetSkipsPerEnv(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix1 := commitFile(t, "b.txt", "two", "fix one") + fix2 := commitFile(t, "c.txt", "three", "fix two") + + // test already has fix1 in patches; staging has neither. Both commits are + // requested. Per (commit,env): test plans only fix2; staging plans both. + manifest := writeManifestFull(t, []string{"dev", "test", "staging", "prod"}, map[string]envSpec{ + "dev": {sha: fix2}, + "test": {sha: base, patches: []string{fix1}}, + "staging": {sha: base}, + "prod": {sha: base}, + }) + + p := newPlanner(t, manifest) + res, err := p.PlanChain([]string{fix1, fix2}, "staging") + if err != nil { + t.Fatalf("PlanChain: %v", err) + } + test := res.Envs[0] + if test.Env != "test" || test.NoOp { + t.Fatalf("test env should still plan a partial set, got %+v", test) + } + if len(test.Commits) != 1 || test.Commits[0] != fix2 { + t.Errorf("test commits = %v, want [%s] (fix1 already applied)", test.Commits, fix2) + } + staging := res.Envs[1] + if len(staging.Commits) != 2 { + t.Errorf("staging commits = %v, want both", staging.Commits) + } +} + +func TestChainGHAOutputs_CarriesSequence(t *testing.T) { + result := &PlanChainResult{ + Envs: []EnvPlan{ + {Env: "test", Branch: "env/test", BaseSHA: "base", Commits: []string{"aaa", "bbb"}}, + {Env: "staging", Branch: "env/staging", BaseSHA: "base", Commits: nil, NoOp: true}, + }, + } + + simple, _ := chainGHAOutputs(result) + + want := map[string]string{ + "env_sequence": "test,staging", + "env_count": "2", + "commits_test": "aaa,bbb", + "no_op_test": "false", + "conflict_expected_test": "false", + "commits_staging": "", + "no_op_staging": "true", + "conflict_expected_staging": "false", + } + for k, v := range want { + if got := simple[k]; got != v { + t.Errorf("chainGHAOutputs[%q] = %q, want %q", k, got, v) + } + } +} + +func TestPlanChain_RejectsNonTrunkCommit(t *testing.T) { + newScratchRepo(t) + base := commitFile(t, "a.txt", "one", "first") + fix1 := commitFile(t, "b.txt", "two", "fix one") + runGit(t, "checkout", "-b", "side") + side := commitFile(t, "d.txt", "four", "side fix") + runGit(t, "checkout", "main") + + manifest := writeManifestFull(t, []string{"dev", "test", "prod"}, map[string]envSpec{ + "dev": {sha: fix1}, + "test": {sha: base}, + "prod": {sha: base}, + }) + + p := newPlanner(t, manifest) + _, err := p.PlanChain([]string{fix1, side}, "test") + if err == nil { + t.Fatal("expected error citing the non-trunk ref") + } + if !strings.Contains(err.Error(), "trunk") { + t.Errorf("error %q should mention trunk", err.Error()) + } +} diff --git a/internal/hotfix/command.go b/internal/hotfix/command.go index a8d777b..e5d62d9 100644 --- a/internal/hotfix/command.go +++ b/internal/hotfix/command.go @@ -128,6 +128,7 @@ func newPlanCommand() *cobra.Command { configPath string manifestKey string commitRef string + commitsRef string targetEnv string actor string remote string @@ -172,6 +173,28 @@ With --dry-run nothing is mutated (the env branch is planned but not created).`, return err } + // --commits selects the multi-commit, multi-env chain path. It is + // additive: --commit remains the single-commit, single-env verb. + if commitsRef != "" { + refs, err := parseCommitRefs(commitsRef) + if err != nil { + return err + } + chain, err := planner.PlanChain(refs, targetEnv) + if err != nil { + return err + } + switch { + case ghaOutput: + return writePlanChainGHAOutput(chain) + case jsonOutput: + return outputJSON(chain) + default: + printPlanChain(chain) + return nil + } + } + result, err := planner.Plan(commitRef, targetEnv) if err != nil { return err @@ -191,7 +214,8 @@ With --dry-run nothing is mutated (the env branch is planned but not created).`, 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(&commitRef, "commit", "", "Trunk commit (SHA or ref) carrying the fix (required)") + cmd.Flags().StringVar(&commitRef, "commit", "", "Trunk commit (SHA or ref) carrying the fix (single-env path)") + cmd.Flags().StringVar(&commitsRef, "commits", "", "Comma-delimited trunk commits to elevate across the env chain up to --target-env") cmd.Flags().StringVar(&targetEnv, "target-env", "", "Environment to hotfix (required)") cmd.Flags().StringVar(&actor, "actor", "", "Actor recorded on the plan (default: $GITHUB_ACTOR)") cmd.Flags().StringVar(&remote, "remote", defaultRemote, "Git remote env branches live on") @@ -200,7 +224,8 @@ With --dry-run nothing is mutated (the env branch is planned but not created).`, cmd.Flags().BoolVar(&jsonOutput, "json", false, "Output the plan as JSON") cmd.Flags().BoolVar(&ghaOutput, "gha-output", false, "Write outputs to $GITHUB_OUTPUT for workflow consumption") - _ = cmd.MarkFlagRequired("commit") + cmd.MarkFlagsMutuallyExclusive("commit", "commits") + cmd.MarkFlagsOneRequired("commit", "commits") _ = cmd.MarkFlagRequired("target-env") return cmd @@ -253,6 +278,64 @@ func writePlanGHAOutput(result *PlanResult) error { return w.Flush() } +// chainGHAOutputs renders a PlanChainResult into a deterministic, additive set +// of GHA output keys describing the bottom-up env sequence and the per-env +// commit lists. Keys are independent of the single-env writePlanGHAOutput keys +// so both can be emitted from one plan run without collision. +// +// - env_sequence: comma-joined bottom-up env list +// - env_count: number of envs in the chain +// - commits_: comma-joined fix SHAs still to apply for that env +// - no_op_: whether that env's whole requested set is already present +// - conflict_expected_: best-effort cherry-pick conflict hint +func chainGHAOutputs(result *PlanChainResult) (simple map[string]string, multiline map[string]string) { + simple = make(map[string]string) + multiline = make(map[string]string) + + envNames := make([]string, 0, len(result.Envs)) + for _, ep := range result.Envs { + envNames = append(envNames, ep.Env) + simple["commits_"+ep.Env] = strings.Join(ep.Commits, ",") + simple["no_op_"+ep.Env] = fmt.Sprintf("%v", ep.NoOp) + simple["conflict_expected_"+ep.Env] = fmt.Sprintf("%v", ep.ConflictExpected) + } + simple["env_sequence"] = strings.Join(envNames, ",") + simple["env_count"] = fmt.Sprintf("%d", len(envNames)) + return simple, multiline +} + +// writePlanChainGHAOutput emits the chain plan as additive GHA outputs. It does +// not remove or overwrite the single-env writePlanGHAOutput keys. +func writePlanChainGHAOutput(result *PlanChainResult) error { + w := ghaoutput.New() + simple, multiline := chainGHAOutputs(result) + for k, v := range simple { + w.Set(k, v) + } + for k, v := range multiline { + w.SetMultiline(k, v) + } + return w.Flush() +} + +// printPlanChain renders the human-readable chain plan: the bottom-up env +// sequence and, per env, the commits still to apply (or a no-op marker). +func printPlanChain(result *PlanChainResult) { + fmt.Printf("Env chain: %d environment(s), bottom-up\n", len(result.Envs)) + for _, ep := range result.Envs { + if ep.NoOp { + fmt.Printf(" %-10s no-op (all requested commits already present)\n", ep.Env) + continue + } + shorts := make([]string, 0, len(ep.Commits)) + for _, c := range ep.Commits { + shorts = append(shorts, short(c)) + } + fmt.Printf(" %-10s %s (base %s): apply %s\n", + ep.Env, ep.Branch, short(ep.BaseSHA), strings.Join(shorts, ", ")) + } +} + func outputJSON(v any) error { data, err := json.MarshalIndent(v, "", " ") if err != nil { diff --git a/internal/hotfix/plan.go b/internal/hotfix/plan.go index c7ab23d..50162a7 100644 --- a/internal/hotfix/plan.go +++ b/internal/hotfix/plan.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/stablekernel/cascade/internal/config" - "github.com/stablekernel/cascade/internal/git" "github.com/stablekernel/cascade/internal/version" ) @@ -225,24 +224,14 @@ func (p *Planner) Plan(fixRef, targetEnv string) (*PlanResult, error) { return nil, fmt.Errorf("manifest has no config block") } - // Resolve the fix to a full SHA up front. - fixSHA, err := p.gitRunner.ResolveSHA(fixRef) + // 1. Resolve the fix and enforce the trunk-ancestry gate over the (single) + // requested ref. resolveTrunkAncestors handles resolution and the per-ref + // ancestry check so the single-env and chain paths share one gate. + shas, err := p.resolveTrunkAncestors([]string{fixRef}) if err != nil { - return nil, fmt.Errorf("resolving fix commit %q: %w", fixRef, err) - } - - // 1. Trunk-ancestry gate: the fix must be an ancestor of trunk tip. - trunkSHA, err := p.gitRunner.ResolveSHA("HEAD") - if err != nil { - return nil, fmt.Errorf("resolving trunk tip: %w", err) - } - onTrunk, err := git.IsAncestor(fixSHA, trunkSHA) - if err != nil { - return nil, fmt.Errorf("checking trunk ancestry: %w", err) - } - if !onTrunk { - return nil, fmt.Errorf("commit %s is not on trunk: a hotfix must apply a commit that is already an ancestor of trunk; merge it to trunk first", short(fixSHA)) + return nil, err } + fixSHA := shas[0] // 2. Target-env eligibility. if cfg.GetEnvironmentIndex(targetEnv) == -1 { @@ -269,8 +258,12 @@ func (p *Planner) Plan(fixRef, targetEnv string) (*PlanResult, error) { DryRun: p.dryRun, } - // 3. No-op check: fix already contained in the target state SHA. - already, err := git.IsAncestor(fixSHA, baseSHA) + // 3. No-op check: the fix is already present in the target when it is an + // ancestor of the recorded state SHA OR it is already recorded in the env's + // applied-patch list. Consulting state.Patches closes the gap where a prior + // hotfix applied the commit onto the diverged env branch without advancing + // state.SHA, which a pure ancestry check would miss and replan. + already, err := commitPresentInEnv(fixSHA, baseSHA, state.Patches) if err != nil { return nil, fmt.Errorf("checking whether fix is already in %q: %w", targetEnv, err) }