From b2ad299457356f72c516b75571a32ed9916611d1 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Sun, 28 Jun 2026 12:15:55 -0400 Subject: [PATCH] feat(status): add consistency --fix to delete orphan env branches Signed-off-by: Joshua Temple --- docs/src/content/docs/coverage-matrix.md | 2 +- internal/hotfix/lifecycle.go | 25 +++++ internal/hotfix/lifecycle_test.go | 95 +++++++++++++++++ internal/status/consistency.go | 94 ++++++++++++++--- internal/status/consistency_test.go | 128 ++++++++++++++++++++++- 5 files changed, 323 insertions(+), 21 deletions(-) diff --git a/docs/src/content/docs/coverage-matrix.md b/docs/src/content/docs/coverage-matrix.md index f401005f..5eb0efc7 100644 --- a/docs/src/content/docs/coverage-matrix.md +++ b/docs/src/content/docs/coverage-matrix.md @@ -159,7 +159,7 @@ not a coverage gap. | `cascade verify` (drift) | unit plus every harness scenario | `internal/verify` | Committed workflows match manifest-regenerated bytes; drift exits non-zero | | `cascade plan` (diff preview) | unit plus CLI | `33-plan-diff`, `internal/plan` | The per-file unified diff is produced without writing files | | `parse-config`, `schema`, `next-version`, `detect-changes`, `generate-changelog` | unit | per-package tests | Pure logic: parsing, version calculation, change detection, changelog assembly | -| `cascade status` and `status consistency` | unit plus harness | `27-verify-orphan`, `internal/status` | State is reported and orphan env branches are flagged | +| `cascade status` and `status consistency` | unit plus harness | `27-verify-orphan`, `internal/status` | State is reported and orphan env branches are flagged, and deleted on the remote with `--fix` | | `branch-protection` and `environments` emit | unit plus CLI | `internal/branchprotection`, `internal/environments` | The JSON body and env config are emitted for the operator (applying them is GitHub-side) | ## The real-GitHub platform ceiling diff --git a/internal/hotfix/lifecycle.go b/internal/hotfix/lifecycle.go index 46c5fdb9..27353c62 100644 --- a/internal/hotfix/lifecycle.go +++ b/internal/hotfix/lifecycle.go @@ -1,6 +1,7 @@ package hotfix import ( + "fmt" "strings" "github.com/stablekernel/cascade/internal/config" @@ -37,6 +38,30 @@ func OrphanEnvBranches(branches []string, state map[string]*config.EnvState) []s return orphans } +// HealOrphanEnvBranches deletes every env/* branch that OrphanEnvBranches flags +// as having no matching divergence, calling del to remove each branch on remote. +// In production del is git.DeleteRemoteBranch, whose delete of an absent branch +// is a no-op success, so HealOrphanEnvBranches is idempotent: re-running it, or +// running it against an orphan that is already gone, deletes nothing further and +// never errors. +// +// Only orphans are deleted. A branch backing a diverged environment is never +// touched, because the deletion set comes from OrphanEnvBranches, which excludes +// it by the same IsDiverged() predicate the hotfix preflight and status +// consistency classify on. The returned slice lists the branches deleted, in +// input order, and is nil when nothing was orphaned. On the first delete error +// the heal stops and returns that error with a nil healed slice. +func HealOrphanEnvBranches(branches []string, state map[string]*config.EnvState, remote string, del func(remote, branch string) error) ([]string, error) { + var healed []string + for _, branch := range OrphanEnvBranches(branches, state) { + if err := del(remote, branch); err != nil { + return nil, fmt.Errorf("deleting orphan branch %s on %s: %w", branch, remote, err) + } + healed = append(healed, branch) + } + return healed, nil +} + // HotfixTagsForBase returns the hotfix tags in tags that belong to the rc base // of baseVersion. A hotfix tag has the dotted shape vX.Y.Z-rc.N.hotfix.M; it // shares the rc base (vX.Y.Z-rc.N) of the version the environment held while diff --git a/internal/hotfix/lifecycle_test.go b/internal/hotfix/lifecycle_test.go index cd5ff80e..d79bec7b 100644 --- a/internal/hotfix/lifecycle_test.go +++ b/internal/hotfix/lifecycle_test.go @@ -75,6 +75,101 @@ func TestOrphanEnvBranches(t *testing.T) { } } +func TestHealOrphanEnvBranches(t *testing.T) { + state := map[string]*config.EnvState{ + // "test" is legitimately diverged: env/test backs it and must never be + // deleted by a heal. + "test": {Ref: "env/test", BaseSHA: "base", Patches: []string{"p1"}}, + // "uat" is not diverged: env/uat is an orphan. + "uat": {SHA: "abc"}, + } + + t.Run("deletes only orphans and leaves diverged branches intact", func(t *testing.T) { + var deleted []string + del := func(remote, branch string) error { + deleted = append(deleted, branch) + return nil + } + + healed, err := HealOrphanEnvBranches( + []string{"main", "env/test", "env/uat", "env/staging"}, state, "origin", del) + if err != nil { + t.Fatalf("HealOrphanEnvBranches() error = %v", err) + } + + want := []string{"env/uat", "env/staging"} + if !reflect.DeepEqual(healed, want) { + t.Fatalf("healed = %v, want %v", healed, want) + } + if !reflect.DeepEqual(deleted, want) { + t.Fatalf("deleted = %v, want %v", deleted, want) + } + for _, b := range deleted { + if b == "env/test" { + t.Fatalf("diverged env branch env/test must never be deleted") + } + } + }) + + t.Run("idempotent when an orphan is already absent", func(t *testing.T) { + // A deleter that no-ops on a missing branch (git.DeleteRemoteBranch's + // real behavior) keeps the heal a clean, repeatable no-op. + calls := 0 + del := func(remote, branch string) error { + calls++ + return nil // already gone is success + } + + first, err := HealOrphanEnvBranches([]string{"env/uat"}, state, "origin", del) + if err != nil { + t.Fatalf("first heal error = %v", err) + } + second, err := HealOrphanEnvBranches([]string{"env/uat"}, state, "origin", del) + if err != nil { + t.Fatalf("second heal error = %v", err) + } + if !reflect.DeepEqual(first, []string{"env/uat"}) || !reflect.DeepEqual(second, []string{"env/uat"}) { + t.Fatalf("heal not idempotent: first=%v second=%v", first, second) + } + if calls != 2 { + t.Fatalf("expected two delete attempts, got %d", calls) + } + }) + + t.Run("no orphans is a clean no-op", func(t *testing.T) { + del := func(remote, branch string) error { + t.Fatalf("delete must not be called when nothing is orphaned") + return nil + } + healed, err := HealOrphanEnvBranches([]string{"main", "env/test"}, state, "origin", del) + if err != nil { + t.Fatalf("error = %v", err) + } + if healed != nil { + t.Fatalf("healed = %v, want nil", healed) + } + }) + + t.Run("propagates a delete error with the healed-so-far set", func(t *testing.T) { + del := func(remote, branch string) error { + return errBoom + } + healed, err := HealOrphanEnvBranches([]string{"env/uat"}, state, "origin", del) + if err == nil { + t.Fatalf("expected error, got nil") + } + if healed != nil { + t.Fatalf("healed = %v, want nil on first-delete failure", healed) + } + }) +} + +var errBoom = errBoomType("boom") + +type errBoomType string + +func (e errBoomType) Error() string { return string(e) } + func TestHotfixTagsForBase(t *testing.T) { tests := []struct { name string diff --git a/internal/status/consistency.go b/internal/status/consistency.go index 6a7eccbd..267888cc 100644 --- a/internal/status/consistency.go +++ b/internal/status/consistency.go @@ -10,60 +10,113 @@ import ( ) // branchLister returns the branch names to check for orphans. The default lists -// the origin remote's branches; tests inject a fixed set so the consistency +// the chosen remote's branches; tests inject a fixed set so the consistency // check is exercised without a real repository. type branchLister func() ([]string, error) -// defaultBranchLister lists the origin remote's branches. -func defaultBranchLister() ([]string, error) { - return git.ListRemoteBranches("origin") +// branchDeleter removes branch on remote. The default is git.DeleteRemoteBranch, +// whose delete of an absent branch is a no-op so --fix stays idempotent; tests +// inject a recording stub so deletions are asserted without a real repository. +type branchDeleter func(remote, branch string) error + +// remoteBranchLister lists the named remote's branches. +func remoteBranchLister(remote string) branchLister { + return func() ([]string, error) { + return git.ListRemoteBranches(remote) + } +} + +// consistencyOptions carries the inputs of the consistency check so the flag +// wiring and the testable core agree on one shape as the surface grows. +type consistencyOptions struct { + configPath string + key string + jsonOutput bool + fix bool + remote string + lister branchLister + deleter branchDeleter } // newConsistencyCommand creates the 'status consistency' subcommand. It reports // env/* integration branches that have no matching divergence in the manifest: // a hotfix leaves an env/ branch only while state[] stays diverged, // so a branch with no diverged env behind it is an orphan from an interrupted -// hotfix or manual meddling and is surfaced here for cleanup. +// hotfix or manual meddling and is surfaced here for cleanup. With --fix it also +// deletes those orphans on the remote so the operator can self-serve the cleanup +// the next hotfix preflight would otherwise demand by hand. func newConsistencyCommand(configPath, key *string, jsonOutput *bool) *cobra.Command { + var fix bool + var remote string + cmd := &cobra.Command{ Use: "consistency", - Short: "Flag env/* branches with no matching manifest divergence", + Short: "Flag (and with --fix, delete) env/* branches with no matching manifest divergence", Long: `Check for orphan integration branches. A hotfix creates an env/ branch that exists only while the environment is diverged. When the environment rejoins trunk the branch is deleted. This command flags any env/ branch that has no matching divergence in the manifest, which -indicates an interrupted hotfix or manual branch creation that should be cleaned up.`, +indicates an interrupted hotfix or manual branch creation that should be cleaned up. + +By default it only reports. With --fix it deletes each flagged orphan branch on the +remote (default origin). A branch that backs a diverged environment is never touched. +Deleting an already-absent branch is a no-op, so --fix is safe to re-run.`, RunE: func(cmd *cobra.Command, args []string) error { - return runConsistency(*configPath, *key, *jsonOutput, defaultBranchLister) + return runConsistency(consistencyOptions{ + configPath: *configPath, + key: *key, + jsonOutput: *jsonOutput, + fix: fix, + remote: remote, + lister: remoteBranchLister(remote), + deleter: git.DeleteRemoteBranch, + }) }, } + + cmd.Flags().BoolVar(&fix, "fix", false, "Delete the flagged orphan env/* branches on the remote") + cmd.Flags().StringVar(&remote, "remote", "origin", "Remote to inspect and, with --fix, delete orphan branches on") + return cmd } -// consistencyOutput is the JSON shape for the consistency command. +// consistencyOutput is the JSON shape for the consistency command. HealedEnvBranches +// is omitted in report-only runs so the default output is unchanged; with --fix it +// lists the orphans deleted so automation can consume what was cleaned up. type consistencyOutput struct { OrphanEnvBranches []string `json:"orphan_env_branches"` + HealedEnvBranches []string `json:"healed_env_branches,omitempty"` } -// runConsistency loads the manifest, lists branches via lister, and reports the -// orphan env/* branches. It is the testable core of the consistency subcommand; -// the branch lister is injected so the check runs without a repository in tests. -func runConsistency(configPath, key string, jsonOutput bool, lister branchLister) error { - file, err := loadManifest(configPath, key) +// runConsistency loads the manifest, lists branches via opts.lister, and reports +// the orphan env/* branches. With opts.fix it deletes those orphans on opts.remote +// through opts.deleter and reports what was healed. It is the testable core of the +// consistency subcommand; the lister and deleter are injected so the check runs +// without a repository in tests. +func runConsistency(opts consistencyOptions) error { + file, err := loadManifest(opts.configPath, opts.key) if err != nil { return err } - branches, err := lister() + branches, err := opts.lister() if err != nil { return fmt.Errorf("listing branches: %w", err) } orphans := hotfix.OrphanEnvBranches(branches, file.State) - if jsonOutput { - return printJSON(consistencyOutput{OrphanEnvBranches: orphans}) + var healed []string + if opts.fix { + healed, err = hotfix.HealOrphanEnvBranches(branches, file.State, opts.remote, opts.deleter) + if err != nil { + return fmt.Errorf("healing orphan branches: %w", err) + } + } + + if opts.jsonOutput { + return printJSON(consistencyOutput{OrphanEnvBranches: orphans, HealedEnvBranches: healed}) } if len(orphans) == 0 { @@ -75,5 +128,12 @@ func runConsistency(configPath, key string, jsonOutput bool, lister branchLister for _, b := range orphans { fmt.Printf(" %s\n", b) } + + if opts.fix { + fmt.Printf("healed env/* branches (deleted on %s):\n", opts.remote) + for _, b := range healed { + fmt.Printf(" %s\n", b) + } + } return nil } diff --git a/internal/status/consistency_test.go b/internal/status/consistency_test.go index 8be76144..a4cc6404 100644 --- a/internal/status/consistency_test.go +++ b/internal/status/consistency_test.go @@ -30,6 +30,15 @@ func writeConsistencyManifest(t *testing.T) string { return path } +// failingDeleter fails the test if called: report-only runs must never delete. +func failingDeleter(t *testing.T) branchDeleter { + t.Helper() + return func(remote, branch string) error { + t.Fatalf("deleter must not be called without --fix (remote=%s branch=%s)", remote, branch) + return nil + } +} + func TestConsistency_OrphanEnvBranchFlagged(t *testing.T) { path := writeConsistencyManifest(t) @@ -40,7 +49,13 @@ func TestConsistency_OrphanEnvBranchFlagged(t *testing.T) { } out := captureOutput(t, func() { - err := runConsistency(path, "ci", false, brancher) + err := runConsistency(consistencyOptions{ + configPath: path, + key: "ci", + remote: "origin", + lister: brancher, + deleter: failingDeleter(t), + }) require.NoError(t, err) }) @@ -56,7 +71,13 @@ func TestConsistency_NoOrphans(t *testing.T) { } out := captureOutput(t, func() { - err := runConsistency(path, "ci", false, brancher) + err := runConsistency(consistencyOptions{ + configPath: path, + key: "ci", + remote: "origin", + lister: brancher, + deleter: failingDeleter(t), + }) require.NoError(t, err) }) @@ -71,11 +92,112 @@ func TestConsistency_JSON(t *testing.T) { } out := captureOutput(t, func() { - err := runConsistency(path, "ci", true, brancher) + err := runConsistency(consistencyOptions{ + configPath: path, + key: "ci", + jsonOutput: true, + remote: "origin", + lister: brancher, + deleter: failingDeleter(t), + }) require.NoError(t, err) }) assert.Contains(t, out, `"orphan_env_branches"`) assert.Contains(t, out, "env/staging") assert.NotContains(t, out, "env/test") + // Report-only default is unchanged: no healed key is emitted. + assert.NotContains(t, out, "healed_env_branches") +} + +func TestConsistency_Fix_DeletesOnlyOrphans(t *testing.T) { + path := writeConsistencyManifest(t) + + brancher := func() ([]string, error) { + return []string{"main", "env/test", "env/staging"}, nil + } + + var deleted []string + deleter := func(remote, branch string) error { + assert.Equal(t, "origin", remote, "deletion must target the inspected remote") + deleted = append(deleted, branch) + return nil + } + + out := captureOutput(t, func() { + err := runConsistency(consistencyOptions{ + configPath: path, + key: "ci", + fix: true, + remote: "origin", + lister: brancher, + deleter: deleter, + }) + require.NoError(t, err) + }) + + assert.Equal(t, []string{"env/staging"}, deleted, "only the orphan is deleted") + assert.NotContains(t, deleted, "env/test", "the diverged env's live branch must never be deleted") + assert.Contains(t, out, "healed env/* branches") + assert.Contains(t, out, "env/staging") +} + +func TestConsistency_Fix_Idempotent(t *testing.T) { + path := writeConsistencyManifest(t) + + brancher := func() ([]string, error) { + return []string{"env/test", "env/staging"}, nil + } + + // A deleter that no-ops on an absent branch mirrors git.DeleteRemoteBranch, + // so re-running --fix (or fixing an already-gone orphan) stays a clean no-op. + calls := 0 + deleter := func(remote, branch string) error { + calls++ + return nil + } + + run := func() { + err := runConsistency(consistencyOptions{ + configPath: path, + key: "ci", + fix: true, + remote: "origin", + lister: brancher, + deleter: deleter, + }) + require.NoError(t, err) + } + + captureOutput(t, run) + captureOutput(t, run) + + assert.Equal(t, 2, calls, "each run heals the same single orphan without error") +} + +func TestConsistency_Fix_JSONReflectsDeletions(t *testing.T) { + path := writeConsistencyManifest(t) + + brancher := func() ([]string, error) { + return []string{"env/test", "env/staging"}, nil + } + deleter := func(remote, branch string) error { return nil } + + out := captureOutput(t, func() { + err := runConsistency(consistencyOptions{ + configPath: path, + key: "ci", + jsonOutput: true, + fix: true, + remote: "origin", + lister: brancher, + deleter: deleter, + }) + require.NoError(t, err) + }) + + assert.Contains(t, out, `"orphan_env_branches"`) + assert.Contains(t, out, `"healed_env_branches"`) + assert.Contains(t, out, "env/staging") + assert.NotContains(t, out, "env/test", "the diverged env branch is neither orphaned nor healed") }