diff --git a/cmd/adopt.go b/cmd/adopt.go index da8c49d..69f00ce 100644 --- a/cmd/adopt.go +++ b/cmd/adopt.go @@ -36,13 +36,13 @@ func runAdopt(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - g := git.New(cwd) - // Parent is the required positional argument parent := args[0] diff --git a/cmd/adopt_test.go b/cmd/adopt_test.go index 9f71a80..9781c5e 100644 --- a/cmd/adopt_test.go +++ b/cmd/adopt_test.go @@ -12,8 +12,8 @@ import ( func TestAdoptBranch(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -40,8 +40,8 @@ func TestAdoptBranch(t *testing.T) { func TestAdoptRejectsAlreadyTracked(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -60,8 +60,8 @@ func TestAdoptRejectsAlreadyTracked(t *testing.T) { func TestAdoptRejectsUntrackedParent(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -81,8 +81,8 @@ func TestAdoptRejectsUntrackedParent(t *testing.T) { func TestAdoptDetectsCycle(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -117,8 +117,8 @@ func TestAdoptDetectsCycle(t *testing.T) { func TestAdoptStoresForkPoint(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) diff --git a/cmd/cascade.go b/cmd/cascade.go index e43a139..b19d0c5 100644 --- a/cmd/cascade.go +++ b/cmd/cascade.go @@ -47,13 +47,13 @@ func runCascade(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - g := git.New(cwd) - // Check if cascade already in progress if state.Exists(g.GetGitDir()) { return errors.New("operation already in progress; use 'gh stack continue' or 'gh stack abort'") diff --git a/cmd/continue.go b/cmd/continue.go index d1bf035..d98640a 100644 --- a/cmd/continue.go +++ b/cmd/continue.go @@ -63,7 +63,7 @@ func runContinue(cmd *cobra.Command, args []string) error { fmt.Printf("%s Completed %s\n", s.SuccessIcon(), s.Branch(st.Current)) - cfg, err := config.Load(cwd) + cfg, err := config.New(g) if err != nil { return err } diff --git a/cmd/create.go b/cmd/create.go index fa2aa58..08bcdf0 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -39,13 +39,13 @@ func runCreate(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - g := git.New(cwd) - // Get current branch currentBranch, err := g.CurrentBranch() if err != nil { diff --git a/cmd/create_test.go b/cmd/create_test.go index ba23992..ead448c 100644 --- a/cmd/create_test.go +++ b/cmd/create_test.go @@ -14,10 +14,9 @@ import ( func TestCreateFromTrunk(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) - cfg.SetTrunk("main") - g := git.New(dir) + cfg, _ := config.New(g) + cfg.SetTrunk("main") // Simulate what create command does currentBranch, _ := g.CurrentBranch() @@ -57,8 +56,8 @@ func TestCreateFromTrunk(t *testing.T) { func TestCreateFromTrackedBranch(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) currentBranch, _ := g.CurrentBranch() cfg.SetTrunk(currentBranch) @@ -86,8 +85,8 @@ func TestCreateFromTrackedBranch(t *testing.T) { func TestCreateRejectsUntrackedBranch(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -119,8 +118,8 @@ func TestCreateRejectsUntrackedBranch(t *testing.T) { func TestCreateWithStagedChanges(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -151,8 +150,8 @@ func TestCreateWithStagedChanges(t *testing.T) { func TestCreateEmptyWithStagedChanges(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -197,8 +196,8 @@ func TestBranchAlreadyExists(t *testing.T) { func TestCreateStoresForkPoint(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) diff --git a/cmd/doctor.go b/cmd/doctor.go new file mode 100644 index 0000000..f6a09f5 --- /dev/null +++ b/cmd/doctor.go @@ -0,0 +1,281 @@ +// cmd/doctor.go +package cmd + +import ( + "fmt" + "os" + "strings" + "time" + + "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" + "github.com/boneskull/gh-stack/internal/health" + "github.com/boneskull/gh-stack/internal/style" + "github.com/spf13/cobra" +) + +var doctorCmd = &cobra.Command{ + Use: "doctor", + Short: "Check stack health and detect stale fork points", + Long: `Diagnose (and optionally repair) stale fork points caused by manual rebases or resets.`, + RunE: runDoctor, +} + +var doctorFixFlag bool + +func init() { + doctorCmd.Flags().BoolVar(&doctorFixFlag, "fix", false, "repair detected issues") + rootCmd.AddCommand(doctorCmd) +} + +// branchResult tracks the outcome of checking a single branch. +type branchResult struct { + name string + healthy bool + issues []string + fixed bool + fixMsg string +} + +func runDoctor(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + + g := git.New(cwd) + + cfg, err := config.New(g) + if err != nil { + return err + } + + if _, err := cfg.GetTrunk(); err != nil { + return err + } + s := style.New() + + branches, err := cfg.ListTrackedBranches() + if err != nil { + return err + } + + if len(branches) == 0 { + fmt.Println("No tracked branches found.") + return nil + } + + fmt.Println(s.Bold("Stack Health Report")) + fmt.Println() + + var issueCount int + for _, branch := range branches { + result := checkBranch(g, cfg, s, branch, doctorFixFlag) + if !printResult(s, result) { + issueCount++ + } + } + + fmt.Println() + printSummary(s, issueCount, doctorFixFlag) + + return nil +} + +// printResult prints the outcome of a single branch check. +// Returns true if the branch is healthy (or was fixed), false if issues remain. +func printResult(s *style.Style, r branchResult) bool { + if r.healthy { + fmt.Printf("%s %s %s\n", s.SuccessIcon(), s.Branch(r.name), s.Muted("(healthy)")) + return true + } + if r.fixed { + fmt.Printf("%s %s: %s\n", s.SuccessIcon(), s.Branch(r.name), r.fixMsg) + return true + } + fmt.Printf("%s %s\n", s.FailureIcon(), s.Branch(r.name)) + for _, issue := range r.issues { + fmt.Printf(" %s\n", issue) + } + return false +} + +// printSummary prints the final summary line after all branches have been checked. +func printSummary(s *style.Style, issueCount int, fix bool) { + if issueCount == 0 { + fmt.Println(s.SuccessMessage("All branches healthy.")) + return + } + noun := "issue" + if issueCount > 1 { + noun = "issues" + } + fmt.Printf("%d %s found.", issueCount, noun) + if !fix { + fmt.Printf(" Run 'gh stack doctor --fix' to repair.") + } + fmt.Println() +} + +func checkBranch(g *git.Git, cfg *config.Config, s *style.Style, branch string, fix bool) branchResult { + result := branchResult{name: branch} + + issues := health.CheckBranch(g, cfg, branch) + if len(issues) == 0 { + result.healthy = true + return result + } + + if !fix { + result.issues = formatIssues(s, issues) + return result + } + + return fixBranch(g, cfg, s, branch, issues) +} + +// formatIssues converts health issues into styled display strings. +func formatIssues(s *style.Style, issues []health.Issue) []string { + out := make([]string, 0, len(issues)) + for _, iss := range issues { + switch iss.Kind { + case health.KindBranchMissing, health.KindParentMissing, health.KindNoForkPoint: + out = append(out, s.Error(iss.Message)) + default: + out = append(out, iss.Message) + } + } + return out +} + +// fixBranch attempts to repair the first issue found by health.CheckBranch. +func fixBranch(g *git.Git, cfg *config.Config, s *style.Style, branch string, issues []health.Issue) branchResult { + result := branchResult{name: branch} + + kind := issues[0].Kind + if !issues[0].Fixable && kind != health.KindDrift { + result.issues = formatIssues(s, issues) + return result + } + + parent, _ := cfg.GetParent(branch) //nolint:errcheck // health check validated parent exists + storedFP, _ := cfg.GetForkPoint(branch) //nolint:errcheck // may be empty for KindNoForkPoint + + switch kind { + case health.KindNoForkPoint: + return fixMissingForkPoint(g, cfg, branch, parent) + case health.KindForkPointMissing, health.KindForkPointNotAncestor: + return fixStaleForkPoint(g, cfg, s, branch, parent, storedFP, issues[0].Message) + case health.KindDrift: + return fixDrift(g, cfg, s, branch, parent, storedFP) + default: + for _, iss := range issues { + result.issues = append(result.issues, iss.Message) + } + return result + } +} + +// fixMissingForkPoint computes and sets a fork point when none is stored. +func fixMissingForkPoint(g *git.Git, cfg *config.Config, branch, parent string) branchResult { + result := branchResult{name: branch} + newFP, err := computeForkPoint(g, parent, branch) + if err != nil { + result.issues = append(result.issues, fmt.Sprintf("No fork point stored and could not compute one: %v", err)) + return result + } + if err := cfg.SetForkPoint(branch, newFP); err != nil { + result.issues = append(result.issues, fmt.Sprintf("Failed to set fork point: %v", err)) + return result + } + result.fixed = true + result.fixMsg = fmt.Sprintf("set fork point to %s", git.AbbrevSHA(newFP)) + return result +} + +// fixStaleForkPoint recomputes and replaces a fork point that is missing or not an ancestor. +func fixStaleForkPoint(g *git.Git, cfg *config.Config, s *style.Style, branch, parent, storedFP, context string) branchResult { + result := branchResult{name: branch} + newFP, err := computeForkPoint(g, parent, branch) + if err != nil { + result.issues = append(result.issues, fmt.Sprintf("%s and could not compute replacement: %v", context, err)) + return result + } + if err := setForkPointWithComment(s, cfg, branch, storedFP, newFP); err != nil { + result.issues = append(result.issues, fmt.Sprintf("Failed to set fork point: %v", err)) + return result + } + result.fixed = true + result.fixMsg = fmt.Sprintf("updated fork point %s \u2192 %s", git.AbbrevSHA(storedFP), git.AbbrevSHA(newFP)) + return result +} + +// fixDrift replaces a stored fork point that doesn't match the computed merge-base. +func fixDrift(g *git.Git, cfg *config.Config, s *style.Style, branch, parent, storedFP string) branchResult { + result := branchResult{name: branch} + mergeBase, err := g.GetMergeBase(parent, branch) + if err != nil { + result.issues = append(result.issues, fmt.Sprintf("Failed to compute merge-base for fix: %v", err)) + return result + } + if err := setForkPointWithComment(s, cfg, branch, storedFP, mergeBase); err != nil { + result.issues = append(result.issues, fmt.Sprintf("Failed to set fork point: %v", err)) + return result + } + result.fixed = true + result.fixMsg = fmt.Sprintf("updated fork point %s \u2192 %s", git.AbbrevSHA(storedFP), git.AbbrevSHA(mergeBase)) + return result +} + +// computeForkPoint tries the reflog-aware fork-point first, falling back to merge-base. +func computeForkPoint(g *git.Git, parent, branch string) (string, error) { + fp, err := g.GetForkPoint(parent, branch) + if err == nil { + return fp, nil + } + return g.GetMergeBase(parent, branch) +} + +// setForkPointWithComment updates the fork point and records the old value +// as an inline git config comment for provenance. Falls back to a plain +// SetForkPoint if the git version doesn't support --comment (requires 2.45+). +func setForkPointWithComment(s *style.Style, cfg *config.Config, branch, oldSHA, newSHA string) error { + comment := fmt.Sprintf("replaces %s (%s)", + git.AbbrevSHA(oldSHA), + time.Now().UTC().Format(time.RFC3339), + ) + err := cfg.SetForkPointWithComment(branch, newSHA, comment) + if err == nil { + return nil + } + + // Only fall back for unsupported --comment flag; propagate real errors. + if !isUnsupportedCommentErr(err) { + return err + } + if setErr := cfg.SetForkPoint(branch, newSHA); setErr != nil { + return setErr + } + warnOldGit(s) + return nil +} + +// isUnsupportedCommentErr returns true if the error indicates that +// git config --comment is not recognized (git < 2.45). +func isUnsupportedCommentErr(err error) bool { + msg := err.Error() + return strings.Contains(msg, "unknown option") +} + +var oldGitWarned bool + +// warnOldGit prints a one-time warning that the user's git version is too old +// for inline config comments. +func warnOldGit(s *style.Style) { + if oldGitWarned { + return + } + oldGitWarned = true + fmt.Printf("%s git config --comment not supported; fix provenance will be missing.\n", s.WarningIcon()) + fmt.Println(s.Muted(" gh-stack works best with git 2.45 or newer.")) +} diff --git a/cmd/init.go b/cmd/init.go index 6953430..e766b46 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -32,13 +32,13 @@ func runInit(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - g := git.New(cwd) - // Determine trunk branch trunk := trunkFlag if trunk == "" { diff --git a/cmd/init_test.go b/cmd/init_test.go index 6f2b2ff..403fbd2 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" ) func setupTestRepo(t *testing.T) string { @@ -37,9 +38,10 @@ func TestInitCommand(t *testing.T) { dir := setupTestRepo(t) // Verify the config package works for init - cfg, err := config.Load(dir) + g := git.New(dir) + cfg, err := config.New(g) if err != nil { - t.Fatalf("Load failed: %v", err) + t.Fatalf("New failed: %v", err) } err = cfg.SetTrunk("main") diff --git a/cmd/link.go b/cmd/link.go index 2d1228e..1728e8f 100644 --- a/cmd/link.go +++ b/cmd/link.go @@ -35,12 +35,12 @@ func runLink(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - - g := git.New(cwd) branch, err := g.CurrentBranch() if err != nil { return err diff --git a/cmd/link_test.go b/cmd/link_test.go index af1ffe1..e2c5ea2 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -11,8 +11,8 @@ import ( func TestLinkPR(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -40,8 +40,8 @@ func TestLinkPR(t *testing.T) { func TestLinkRejectsUntrackedBranch(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -59,8 +59,8 @@ func TestLinkRejectsUntrackedBranch(t *testing.T) { func TestLinkOverwritesPR(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) diff --git a/cmd/log.go b/cmd/log.go index d53c163..2c248e1 100644 --- a/cmd/log.go +++ b/cmd/log.go @@ -40,7 +40,9 @@ func runLog(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } @@ -49,8 +51,6 @@ func runLog(cmd *cobra.Command, args []string) error { if err != nil { return err } - - g := git.New(cwd) currentBranch, _ := g.CurrentBranch() //nolint:errcheck // empty string is fine for display // Try to get GitHub client for PR URLs (optional - may fail if not in a GitHub repo) diff --git a/cmd/log_test.go b/cmd/log_test.go index b39ea73..e1575a3 100644 --- a/cmd/log_test.go +++ b/cmd/log_test.go @@ -5,13 +5,15 @@ import ( "testing" "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" "github.com/boneskull/gh-stack/internal/tree" ) func TestLogBuildTree(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) + g := git.New(dir) + cfg, _ := config.New(g) cfg.SetTrunk("main") cfg.SetParent("feature-a", "main") cfg.SetParent("feature-b", "feature-a") @@ -50,7 +52,8 @@ func TestLogBuildTree(t *testing.T) { func TestLogEmptyStack(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) + g := git.New(dir) + cfg, _ := config.New(g) cfg.SetTrunk("main") // No branches tracked, just trunk @@ -70,7 +73,8 @@ func TestLogEmptyStack(t *testing.T) { func TestLogMultipleBranches(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) + g := git.New(dir) + cfg, _ := config.New(g) cfg.SetTrunk("main") // Two branches off main cfg.SetParent("feature-a", "main") diff --git a/cmd/orphan.go b/cmd/orphan.go index 66d5aa9..cf4654d 100644 --- a/cmd/orphan.go +++ b/cmd/orphan.go @@ -33,13 +33,13 @@ func runOrphan(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - g := git.New(cwd) - // Determine branch to orphan var branchName string if len(args) > 0 { diff --git a/cmd/orphan_test.go b/cmd/orphan_test.go index 576359b..218a20b 100644 --- a/cmd/orphan_test.go +++ b/cmd/orphan_test.go @@ -12,8 +12,8 @@ import ( func TestOrphanBranch(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -41,8 +41,8 @@ func TestOrphanBranch(t *testing.T) { func TestOrphanRejectsWithChildren(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -66,8 +66,8 @@ func TestOrphanRejectsWithChildren(t *testing.T) { func TestOrphanForceWithDescendants(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -114,8 +114,8 @@ func TestOrphanForceWithDescendants(t *testing.T) { func TestOrphanRemovesPR(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -145,8 +145,8 @@ func TestOrphanRemovesPR(t *testing.T) { func TestOrphanRemovesForkPoint(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) diff --git a/cmd/submit.go b/cmd/submit.go index 843e7f1..f147a51 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -78,13 +78,13 @@ func runSubmit(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - g := git.New(cwd) - // Check if operation already in progress if state.Exists(g.GetGitDir()) { return errors.New("operation already in progress; use 'gh stack continue' or 'gh stack abort'") diff --git a/cmd/sync.go b/cmd/sync.go index 85b2cc3..a8fd23c 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -96,7 +96,9 @@ func runSync(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } @@ -106,8 +108,6 @@ func runSync(cmd *cobra.Command, args []string) error { return err } - g := git.New(cwd) - trunk, err := cfg.GetTrunk() if err != nil { return err diff --git a/cmd/undo.go b/cmd/undo.go index 76ac651..7fd50c2 100644 --- a/cmd/undo.go +++ b/cmd/undo.go @@ -124,7 +124,7 @@ func runUndo(cmd *cobra.Command, args []string) error { } // Load config for restoring stack metadata - cfg, err := config.Load(cwd) + cfg, err := config.New(g) if err != nil { return fmt.Errorf("failed to load config: %w", err) } diff --git a/cmd/unlink.go b/cmd/unlink.go index e5e6194..1ed18e6 100644 --- a/cmd/unlink.go +++ b/cmd/unlink.go @@ -28,12 +28,12 @@ func runUnlink(cmd *cobra.Command, args []string) error { return err } - cfg, err := config.Load(cwd) + g := git.New(cwd) + + cfg, err := config.New(g) if err != nil { return err } - - g := git.New(cwd) branch, err := g.CurrentBranch() if err != nil { return err diff --git a/cmd/unlink_test.go b/cmd/unlink_test.go index c3d23a9..1bad1be 100644 --- a/cmd/unlink_test.go +++ b/cmd/unlink_test.go @@ -11,8 +11,8 @@ import ( func TestUnlinkPR(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) @@ -41,8 +41,8 @@ func TestUnlinkPR(t *testing.T) { func TestUnlinkIdempotent(t *testing.T) { dir := setupTestRepo(t) - cfg, _ := config.Load(dir) g := git.New(dir) + cfg, _ := config.New(g) trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) diff --git a/e2e/doctor_test.go b/e2e/doctor_test.go new file mode 100644 index 0000000..87a59d2 --- /dev/null +++ b/e2e/doctor_test.go @@ -0,0 +1,184 @@ +// e2e/doctor_test.go +package e2e_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestDoctorHealthyStack(t *testing.T) { + env := NewTestEnv(t) + + env.MustRun("init") + env.MustRun("create", "feature-a") + env.CreateCommit("work on a") + env.MustRun("create", "feature-b") + env.CreateCommit("work on b") + + result := env.MustRun("doctor") + if !result.ContainsStdout("healthy") { + t.Errorf("expected healthy output, got: %s", result.Stdout) + } + if !result.ContainsStdout("All branches healthy") { + t.Errorf("expected 'All branches healthy', got: %s", result.Stdout) + } +} + +func TestDoctorDetectsDrift(t *testing.T) { + env := NewTestEnv(t) + + env.MustRun("init") + env.MustRun("create", "feature-a") + env.CreateCommit("work on a") + env.MustRun("create", "feature-b") + env.CreateCommit("work on b") + + // Move main forward and rebase feature-a to cause stored fork point drift + env.Git("checkout", "feature-a") + env.CreateCommit("extra work on a") + env.Git("checkout", "main") + env.CreateCommit("main moves forward") + env.Git("checkout", "feature-a") + env.Git("rebase", "main") + + // Now the stored fork point is stale + result := env.Run("doctor") + if !result.ContainsStdout("Drift detected") && !result.ContainsStdout("not an ancestor") { + t.Errorf("expected drift or ancestor issue, got: %s", result.Stdout) + } + + // Fix it + result = env.MustRun("doctor", "--fix") + if !result.ContainsStdout("updated fork point") && !result.ContainsStdout("set fork point") { + t.Errorf("expected fix message, got: %s", result.Stdout) + } + + // Should be healthy now + result = env.MustRun("doctor") + if !result.ContainsStdout("All branches healthy") { + t.Errorf("expected all healthy after fix, got: %s", result.Stdout) + } +} + +func TestDoctorMissingBranch(t *testing.T) { + env := NewTestEnv(t) + + env.MustRun("init") + env.MustRun("create", "feature-a") + env.CreateCommit("work on a") + + // Remove the branch ref but keep the stack config entries + env.Git("checkout", "main") + env.Git("update-ref", "-d", "refs/heads/feature-a") + + result := env.Run("doctor") + if !result.ContainsStdout("does not exist locally") { + t.Errorf("expected 'does not exist locally', got: %s", result.Stdout) + } +} + +func TestDoctorMissingParent(t *testing.T) { + env := NewTestEnv(t) + + env.MustRun("init") + env.MustRun("create", "feature-a") + env.CreateCommit("work on a") + env.MustRun("create", "feature-b") + env.CreateCommit("work on b") + + // Delete the parent branch (feature-a) + env.Git("checkout", "main") + env.Git("branch", "-D", "feature-a") + + result := env.Run("doctor") + // feature-b should report its parent missing + if !result.ContainsStdout("does not exist locally") { + t.Errorf("expected parent missing report, got: %s", result.Stdout) + } +} + +func TestDoctorFixUpdatesConfig(t *testing.T) { + env := NewTestEnv(t) + + env.MustRun("init") + env.MustRun("create", "feature-a") + env.CreateCommit("work on a") + + // Record the original fork point + originalFP := strings.TrimSpace(env.Git("config", "--get", "branch.feature-a.stackForkPoint")) + + // Move main forward and rebase feature-a to cause drift + env.Git("checkout", "main") + env.CreateCommit("main moves") + env.Git("checkout", "feature-a") + env.Git("rebase", "main") + + // The stored fork point should still be the old one + storedFP := strings.TrimSpace(env.Git("config", "--get", "branch.feature-a.stackForkPoint")) + if storedFP != originalFP { + t.Fatalf("expected stored fork point to still be %s, got %s", originalFP, storedFP) + } + + // Run doctor --fix + env.MustRun("doctor", "--fix") + + // Read the config directly to verify it changed + newFP := strings.TrimSpace(env.Git("config", "--get", "branch.feature-a.stackForkPoint")) + if newFP == originalFP { + t.Errorf("expected fork point to be updated, but it's still %s", originalFP) + } + + // Verify the new fork point matches the current merge-base + mergeBase := strings.TrimSpace(env.Git("merge-base", "main", "feature-a")) + if newFP != mergeBase { + t.Errorf("expected new fork point %s to match merge-base %s", newFP, mergeBase) + } + + // Verify inline provenance comment was written to the config value line. + // git config --comment (Git 2.45+) appends "# replaces ()" + // on the same line as the stackForkPoint value. + // On older git the comment won't be present; that's fine (doctor falls back gracefully). + configData, err := os.ReadFile(filepath.Join(env.WorkDir, ".git", "config")) + if err != nil { + t.Fatalf("failed to read .git/config: %v", err) + } + configStr := string(configData) + if strings.Contains(configStr, "# replaces") { + abbrevOld := originalFP[:7] + if !strings.Contains(configStr, abbrevOld) { + t.Errorf("expected abbreviated old fork point %s in config comment", abbrevOld) + } + } else { + t.Log("git config --comment not supported; skipping provenance comment assertion") + } +} + +func TestDoctorMissingTrunk(t *testing.T) { + env := NewTestEnv(t) + + env.MustRun("init") + env.MustRun("create", "feature-a") + env.CreateCommit("work on a") + + // Delete the trunk branch (main) while on feature-a + env.Git("branch", "-D", "main") + + result := env.Run("doctor") + if !result.ContainsStdout("Trunk branch") || !result.ContainsStdout("does not exist locally") { + t.Errorf("expected missing trunk report, got: %s", result.Stdout) + } +} + +func TestDoctorNotInitialized(t *testing.T) { + env := NewTestEnv(t) + + result := env.Run("doctor") + if result.Success() { + t.Error("expected doctor to fail before init") + } + if !result.ContainsStderr("not initialized") { + t.Errorf("expected 'not initialized' error, got stderr: %s", result.Stderr) + } +} diff --git a/e2e/helpers_test.go b/e2e/helpers_test.go index 7930c79..723ff69 100644 --- a/e2e/helpers_test.go +++ b/e2e/helpers_test.go @@ -53,7 +53,7 @@ func NewTestEnv(t *testing.T) *TestEnv { BinaryPath: binaryPath, } - env.Git("init") + env.Git("init", "-b", "main") env.Git("config", "user.email", "test@example.com") env.Git("config", "user.name", "Test User") // Prevent git from opening an editor (for commits, rebases, etc.) diff --git a/internal/config/config.go b/internal/config/config.go index 4c93686..ac06d0f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,12 +3,12 @@ package config import ( "bufio" - "bytes" "errors" - "os/exec" "regexp" "strconv" "strings" + + "github.com/boneskull/gh-stack/internal/git" ) // ErrNotInitialized is returned when stack tracking is not initialized. @@ -25,77 +25,76 @@ var ErrNoForkPoint = errors.New("no fork point stored for branch") // Config provides access to stack metadata stored in .git/config. type Config struct { - repoPath string + g *git.Git } -// Load creates a Config for the repository at the given path. -func Load(repoPath string) (*Config, error) { - if _, err := exec.Command("git", "-C", repoPath, "rev-parse", "--git-dir").Output(); err != nil { +// New creates a Config that routes all git-config calls through the given Git instance. +func New(g *git.Git) (*Config, error) { + if _, err := g.GetResolvedGitDir(); err != nil { return nil, errors.New("not a git repository") } - return &Config{repoPath: repoPath}, nil + return &Config{g: g}, nil } // GetTrunk returns the configured trunk branch name. func (c *Config) GetTrunk() (string, error) { - out, err := exec.Command("git", "-C", c.repoPath, "config", "--get", "stack.trunk").Output() + out, err := c.g.ConfigGet("stack.trunk") if err != nil { return "", ErrNotInitialized } - return strings.TrimSpace(string(out)), nil + return out, nil } // SetTrunk sets the trunk branch name. func (c *Config) SetTrunk(branch string) error { - cmd := exec.Command("git", "-C", c.repoPath, "config", "stack.trunk", branch) - return cmd.Run() + return c.g.ConfigSet("stack.trunk", branch) } // GetParent returns the parent branch for the given branch. func (c *Config) GetParent(branch string) (string, error) { key := "branch." + branch + ".stackParent" - out, err := exec.Command("git", "-C", c.repoPath, "config", "--get", key).Output() + out, err := c.g.ConfigGet(key) if err != nil { return "", ErrBranchNotTracked } - return strings.TrimSpace(string(out)), nil + return out, nil } // SetParent sets the parent branch for the given branch. func (c *Config) SetParent(branch, parent string) error { key := "branch." + branch + ".stackParent" - return exec.Command("git", "-C", c.repoPath, "config", key, parent).Run() + return c.g.ConfigSet(key, parent) } // RemoveParent removes the parent tracking for a branch. func (c *Config) RemoveParent(branch string) error { key := "branch." + branch + ".stackParent" // --unset returns error if key doesn't exist, which is fine - _ = exec.Command("git", "-C", c.repoPath, "config", "--unset", key).Run() //nolint:errcheck // unset returns error if key missing + _ = c.g.ConfigUnset(key) //nolint:errcheck // unset returns error if key missing return nil } // GetPR returns the PR number for the given branch. func (c *Config) GetPR(branch string) (int, error) { key := "branch." + branch + ".stackPR" - out, err := exec.Command("git", "-C", c.repoPath, "config", "--get", key).Output() + out, err := c.g.ConfigGet(key) if err != nil { return 0, ErrNoPR } - return strconv.Atoi(strings.TrimSpace(string(out))) + return strconv.Atoi(out) } // SetPR sets the PR number for the given branch. func (c *Config) SetPR(branch string, pr int) error { key := "branch." + branch + ".stackPR" - return exec.Command("git", "-C", c.repoPath, "config", key, strconv.Itoa(pr)).Run() + return c.g.ConfigSet(key, strconv.Itoa(pr)) } // RemovePR removes the PR association for a branch. func (c *Config) RemovePR(branch string) error { key := "branch." + branch + ".stackPR" // --unset returns error if key doesn't exist, which is fine - _ = exec.Command("git", "-C", c.repoPath, "config", "--unset", key).Run() //nolint:errcheck // unset returns error if key missing + _ = c.g.ConfigUnset(key) //nolint:errcheck // unset returns error if key missing return nil } @@ -103,30 +102,39 @@ func (c *Config) RemovePR(branch string) error { // The fork point is where the branch originally diverged from its parent. func (c *Config) GetForkPoint(branch string) (string, error) { key := "branch." + branch + ".stackForkPoint" - out, err := exec.Command("git", "-C", c.repoPath, "config", "--get", key).Output() + out, err := c.g.ConfigGet(key) if err != nil { return "", ErrNoForkPoint } - return strings.TrimSpace(string(out)), nil + return out, nil } // SetForkPoint stores the fork point SHA for a branch. func (c *Config) SetForkPoint(branch, sha string) error { key := "branch." + branch + ".stackForkPoint" - return exec.Command("git", "-C", c.repoPath, "config", key, sha).Run() + return c.g.ConfigSet(key, sha) +} + +// SetForkPointWithComment stores the fork point SHA with an inline comment. +// Requires Git 2.45+. The comment appears after the value on the same line. +// Returns an error with stderr content on failure, so callers can distinguish +// "unknown option" (old git) from other failures. +func (c *Config) SetForkPointWithComment(branch, sha, comment string) error { + key := "branch." + branch + ".stackForkPoint" + return c.g.ConfigSetWithComment(key, sha, comment) } // RemoveForkPoint removes the stored fork point for a branch. func (c *Config) RemoveForkPoint(branch string) error { key := "branch." + branch + ".stackForkPoint" - _ = exec.Command("git", "-C", c.repoPath, "config", "--unset", key).Run() //nolint:errcheck // unset returns error if key missing + _ = c.g.ConfigUnset(key) //nolint:errcheck // unset returns error if key missing return nil } // ListTrackedBranches returns all branches that have a stackParent set. func (c *Config) ListTrackedBranches() ([]string, error) { // Note: git normalizes config keys to lowercase, so stackParent becomes stackparent - out, err := exec.Command("git", "-C", c.repoPath, "config", "--get-regexp", "^branch\\..*\\.stackparent$").Output() + out, err := c.g.ConfigGetRegexp(`^branch\..*\.stackparent$`) if err != nil { // No matches is not an error — git config exits non-zero when no keys match return []string{}, nil //nolint:nilerr // non-zero exit = no matching config keys @@ -134,7 +142,7 @@ func (c *Config) ListTrackedBranches() ([]string, error) { var branches []string re := regexp.MustCompile(`^branch\.(.+)\.stackparent\s+`) - scanner := bufio.NewScanner(bytes.NewReader(out)) + scanner := bufio.NewScanner(strings.NewReader(out)) for scanner.Scan() { line := scanner.Text() if matches := re.FindStringSubmatch(line); len(matches) > 1 { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9b395ca..7adf813 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,13 +3,17 @@ package config_test import ( "errors" + "os" "os/exec" + "path/filepath" + "strings" "testing" "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" ) -func setupTestRepo(t *testing.T) string { +func setupTestRepo(t *testing.T) *git.Git { t.Helper() dir := t.TempDir() @@ -24,15 +28,15 @@ func setupTestRepo(t *testing.T) string { exec.Command("git", "-C", dir, "config", "user.email", "test@test.com").Run() exec.Command("git", "-C", dir, "config", "user.name", "Test").Run() - return dir + return git.New(dir) } func TestGetTrunk_NotInitialized(t *testing.T) { - dir := setupTestRepo(t) + g := setupTestRepo(t) - cfg, err := config.Load(dir) + cfg, err := config.New(g) if err != nil { - t.Fatalf("Load failed: %v", err) + t.Fatalf("New failed: %v", err) } _, err = cfg.GetTrunk() @@ -42,11 +46,11 @@ func TestGetTrunk_NotInitialized(t *testing.T) { } func TestSetTrunk(t *testing.T) { - dir := setupTestRepo(t) + g := setupTestRepo(t) - cfg, err := config.Load(dir) + cfg, err := config.New(g) if err != nil { - t.Fatalf("Load failed: %v", err) + t.Fatalf("New failed: %v", err) } if setErr := cfg.SetTrunk("main"); setErr != nil { @@ -63,9 +67,9 @@ func TestSetTrunk(t *testing.T) { } func TestGetParent_NotTracked(t *testing.T) { - dir := setupTestRepo(t) + g := setupTestRepo(t) - cfg, _ := config.Load(dir) + cfg, _ := config.New(g) _, err := cfg.GetParent("feature-a") if !errors.Is(err, config.ErrBranchNotTracked) { @@ -74,9 +78,9 @@ func TestGetParent_NotTracked(t *testing.T) { } func TestSetAndGetParent(t *testing.T) { - dir := setupTestRepo(t) + g := setupTestRepo(t) - cfg, _ := config.Load(dir) + cfg, _ := config.New(g) cfg.SetTrunk("main") if err := cfg.SetParent("feature-a", "main"); err != nil { @@ -93,9 +97,9 @@ func TestSetAndGetParent(t *testing.T) { } func TestPRNumber(t *testing.T) { - dir := setupTestRepo(t) + g := setupTestRepo(t) - cfg, _ := config.Load(dir) + cfg, _ := config.New(g) // No PR set initially _, err := cfg.GetPR("feature-a") @@ -129,9 +133,9 @@ func TestPRNumber(t *testing.T) { } func TestListTrackedBranches(t *testing.T) { - dir := setupTestRepo(t) + g := setupTestRepo(t) - cfg, _ := config.Load(dir) + cfg, _ := config.New(g) cfg.SetTrunk("main") cfg.SetParent("feature-a", "main") cfg.SetParent("feature-b", "feature-a") @@ -156,8 +160,8 @@ func TestListTrackedBranches(t *testing.T) { } func TestForkPoint(t *testing.T) { - dir := setupTestRepo(t) - cfg, _ := config.Load(dir) + g := setupTestRepo(t) + cfg, _ := config.New(g) // Initially no fork point _, err := cfg.GetForkPoint("feature") @@ -191,3 +195,40 @@ func TestForkPoint(t *testing.T) { t.Errorf("after remove, GetForkPoint = %v, want ErrNoForkPoint", err) } } + +func TestSetForkPointWithComment(t *testing.T) { + g := setupTestRepo(t) + cfg, _ := config.New(g) + + sha := "abc123def456" + comment := "replaces deadbee (2026-02-14T00:00:00Z)" + + if err := cfg.SetForkPointWithComment("feature", sha, comment); err != nil { + // git config --comment requires Git 2.45+; skip on older versions. + t.Skipf("SetForkPointWithComment not supported (git too old?): %v", err) + } + + // Value should be readable via GetForkPoint (git config ignores inline comments) + got, err := cfg.GetForkPoint("feature") + if err != nil { + t.Fatalf("GetForkPoint after SetForkPointWithComment failed: %v", err) + } + if got != sha { + t.Errorf("GetForkPoint = %q, want %q", got, sha) + } + + // The raw config file should contain the inline comment + gitDir, err := g.GetResolvedGitDir() + if err != nil { + t.Fatalf("failed to resolve git dir: %v", err) + } + configPath := filepath.Join(gitDir, "config") + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("failed to read .git/config: %v", err) + } + configStr := string(data) + if !strings.Contains(configStr, "# "+comment) { + t.Errorf("expected inline comment in config, got:\n%s", configStr) + } +} diff --git a/internal/git/git.go b/internal/git/git.go index c71232e..c3c8388 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -525,6 +525,63 @@ func (g *Git) GetCommits(base, head string) ([]Commit, error) { return commits, nil } +// IsAncestor returns true if ancestor is an ancestor of descendant. +// Both arguments must be valid commit references. +func (g *Git) IsAncestor(ancestor, descendant string) (bool, error) { + err := g.runSilent("merge-base", "--is-ancestor", ancestor, descendant) + if err == nil { + return true, nil + } + + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + // Exit status 1 means "not ancestor" when both commits exist. + if exitErr.ExitCode() == 1 { + return false, nil + } + } + + // Propagate other failures (e.g., invalid refs, repo errors). + return false, err +} + +// GetForkPoint returns the reflog-aware fork point of branch from parent. +// This uses `git merge-base --fork-point`, which inspects the reflog to find +// where the branch originally diverged. If the underlying Git command fails +// for any reason, an error is returned and callers may fall back to GetMergeBase. +func (g *Git) GetForkPoint(parent, branch string) (string, error) { + return g.run("merge-base", "--fork-point", parent, branch) +} + +// ConfigGet returns the value of a git config key. +// Returns an error if the key does not exist. +func (g *Git) ConfigGet(key string) (string, error) { + return g.run("config", "--get", key) +} + +// ConfigSet sets a git config key to a value. +func (g *Git) ConfigSet(key, value string) error { + return g.runSilent("config", key, value) +} + +// ConfigSetWithComment sets a git config key to a value with an inline comment. +// Requires Git 2.45+. Returns an error with stderr content on failure, so +// callers can distinguish "unknown option" (old git) from other failures. +func (g *Git) ConfigSetWithComment(key, value, comment string) error { + return g.runSilent("config", "--comment", comment, key, value) +} + +// ConfigUnset removes a git config key. Returns an error if the key does not exist. +func (g *Git) ConfigUnset(key string) error { + return g.runSilent("config", "--unset", key) +} + +// ConfigGetRegexp returns the raw output of `git config --get-regexp`. +// Returns an error if no keys match. +func (g *Git) ConfigGetRegexp(pattern string) (string, error) { + return g.run("config", "--get-regexp", pattern) +} + // AbbrevSHA safely abbreviates a SHA to 7 characters. // Returns the full string if it's shorter than 7 characters. func AbbrevSHA(sha string) string { diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 5fc45a6..2603be8 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -709,6 +709,137 @@ func TestIsRebaseInProgressWorktree(t *testing.T) { } } +// contains checks if substr is in s (simple helper to avoid importing strings in test). +func contains(s, substr string) bool { + return filepath.Base(s) == substr || len(s) >= len(substr) && searchString(s, substr) +} + +func searchString(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} + +func TestIsAncestor(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + // Record the initial commit SHA + initial, _ := g.GetTip("HEAD") + + // Create a linear chain: initial -> A -> B + if err := os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a"), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + if out, err := exec.Command("git", "-C", dir, "add", ".").CombinedOutput(); err != nil { + t.Fatalf("git add failed: %v\n%s", err, out) + } + if out, err := exec.Command("git", "-C", dir, "commit", "-m", "commit A").CombinedOutput(); err != nil { + t.Fatalf("git commit failed: %v\n%s", err, out) + } + shaA, err := g.GetTip("HEAD") + if err != nil { + t.Fatalf("GetTip failed: %v", err) + } + + if err := os.WriteFile(filepath.Join(dir, "b.txt"), []byte("b"), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + if out, err := exec.Command("git", "-C", dir, "add", ".").CombinedOutput(); err != nil { + t.Fatalf("git add failed: %v\n%s", err, out) + } + if out, err := exec.Command("git", "-C", dir, "commit", "-m", "commit B").CombinedOutput(); err != nil { + t.Fatalf("git commit failed: %v\n%s", err, out) + } + shaB, err := g.GetTip("HEAD") + if err != nil { + t.Fatalf("GetTip failed: %v", err) + } + + // initial is ancestor of B + isAnc, err := g.IsAncestor(initial, shaB) + if err != nil { + t.Fatalf("IsAncestor failed: %v", err) + } + if !isAnc { + t.Error("expected initial to be ancestor of B") + } + + // A is ancestor of B + isAnc, err = g.IsAncestor(shaA, shaB) + if err != nil { + t.Fatalf("IsAncestor failed: %v", err) + } + if !isAnc { + t.Error("expected A to be ancestor of B") + } + + // B is NOT ancestor of A + isAnc, err = g.IsAncestor(shaB, shaA) + if err != nil { + t.Fatalf("IsAncestor failed: %v", err) + } + if isAnc { + t.Error("expected B to NOT be ancestor of A") + } + + // A commit is always its own ancestor + isAnc, err = g.IsAncestor(shaA, shaA) + if err != nil { + t.Fatalf("IsAncestor failed: %v", err) + } + if !isAnc { + t.Error("expected A to be ancestor of itself") + } + + // Invalid ref should return an error, not (false, nil) + _, err = g.IsAncestor("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", shaA) + if err == nil { + t.Error("expected error for invalid ancestor ref, got nil") + } + + _, err = g.IsAncestor(shaA, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef") + if err == nil { + t.Error("expected error for invalid descendant ref, got nil") + } +} + +func TestGetForkPoint(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Record the divergence point + diverge, _ := g.GetTip(trunk) + + // Create child branch from trunk + g.CreateAndCheckout("child") + + // Add commits on child + if err := os.WriteFile(filepath.Join(dir, "child.txt"), []byte("child"), 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + if out, err := exec.Command("git", "-C", dir, "add", ".").CombinedOutput(); err != nil { + t.Fatalf("git add failed: %v\n%s", err, out) + } + if out, err := exec.Command("git", "-C", dir, "commit", "-m", "child commit").CombinedOutput(); err != nil { + t.Fatalf("git commit failed: %v\n%s", err, out) + } + + // GetForkPoint should return the divergence point + fp, err := g.GetForkPoint(trunk, "child") + if err != nil { + t.Fatalf("GetForkPoint failed: %v", err) + } + if fp != diverge { + t.Errorf("expected fork point %s, got %s", diverge, fp) + } +} + func TestRemoteBranchExists(t *testing.T) { // Create main repo dir := setupTestRepo(t) diff --git a/internal/health/health.go b/internal/health/health.go new file mode 100644 index 0000000..d6703f2 --- /dev/null +++ b/internal/health/health.go @@ -0,0 +1,103 @@ +// internal/health/health.go +package health + +import ( + "fmt" + + "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" +) + +// IssueKind identifies which health check produced an issue. +type IssueKind int + +const ( + // KindBranchMissing means the branch does not exist locally. + KindBranchMissing IssueKind = iota + 1 + // KindParentMissing means the parent branch does not exist locally. + KindParentMissing + // KindNoForkPoint means no fork point is stored in config. + KindNoForkPoint + // KindForkPointMissing means the stored fork point commit doesn't exist. + KindForkPointMissing + // KindForkPointNotAncestor means the stored fork point is not an ancestor of the branch. + KindForkPointNotAncestor + // KindDrift means the stored fork point doesn't match the computed merge-base. + KindDrift + // KindCheckFailed means a check itself errored (e.g., merge-base computation failed). + KindCheckFailed +) + +// Issue describes a branch health problem. +type Issue struct { + Kind IssueKind // which check produced this issue + Message string // plain text, no ANSI styling + Fixable bool // can `doctor --fix` repair this? +} + +// CheckBranch runs lightweight health checks on a tracked branch. +// Returns nil if healthy. Checks run in order and bail on first blocker: +// 1. Branch exists locally +// 2. Parent exists locally (including trunk) +// 3. Fork point is stored in config +// 4. Fork point commit exists in repo +// 5. Fork point is ancestor of branch +// 6. Stored fork point matches merge-base (drift detection) +func CheckBranch(g *git.Git, cfg *config.Config, branch string) []Issue { + // Check 1: branch exists locally + if !g.BranchExists(branch) { + return []Issue{{Kind: KindBranchMissing, Message: "Branch does not exist locally", Fixable: false}} + } + + // Check 2: parent exists locally + parent, err := cfg.GetParent(branch) + if err != nil { + return []Issue{{Kind: KindParentMissing, Message: "No parent configured", Fixable: false}} + } + + trunk, _ := cfg.GetTrunk() //nolint:errcheck // caller should validate trunk exists + if parent == trunk { + if !g.BranchExists(trunk) { + return []Issue{{Kind: KindParentMissing, Message: fmt.Sprintf("Trunk branch %s does not exist locally", trunk), Fixable: false}} + } + } else if !g.BranchExists(parent) { + return []Issue{{Kind: KindParentMissing, Message: fmt.Sprintf("Parent branch %s does not exist locally", parent), Fixable: false}} + } + + // Check 3: fork point is stored + storedFP, err := cfg.GetForkPoint(branch) + if err != nil { + return []Issue{{Kind: KindNoForkPoint, Message: "No fork point stored", Fixable: true}} + } + + // Check 4: fork point commit exists + if !g.CommitExists(storedFP) { + return []Issue{{Kind: KindForkPointMissing, Message: fmt.Sprintf("Stored fork point %s does not exist", git.AbbrevSHA(storedFP)), Fixable: true}} + } + + // Check 5: fork point is ancestor of branch + isAnc, err := g.IsAncestor(storedFP, branch) + if err != nil { + return []Issue{{Kind: KindCheckFailed, Message: fmt.Sprintf("Failed to check if stored fork point %s is an ancestor of %s: %v", git.AbbrevSHA(storedFP), branch, err), Fixable: false}} + } + if !isAnc { + return []Issue{{Kind: KindForkPointNotAncestor, Message: fmt.Sprintf("Stored fork point %s is not an ancestor of %s", git.AbbrevSHA(storedFP), branch), Fixable: true}} + } + + // Check 6: drift detection + mergeBase, err := g.GetMergeBase(parent, branch) + if err != nil { + return []Issue{{Kind: KindCheckFailed, Message: fmt.Sprintf("Failed to compute merge-base for %s and %s: %v", parent, branch, err), Fixable: false}} + } + + if storedFP != mergeBase { + return []Issue{ + {Kind: KindDrift, Message: fmt.Sprintf("Stored parent: %s", parent), Fixable: false}, + {Kind: KindDrift, Message: fmt.Sprintf("Stored fork: %s", git.AbbrevSHA(storedFP)), Fixable: false}, + {Kind: KindDrift, Message: fmt.Sprintf("Actual fork: %s", git.AbbrevSHA(mergeBase)), Fixable: false}, + {Kind: KindDrift, Message: "Drift detected: stored fork point does not match merge-base", Fixable: true}, + } + } + + return nil +} diff --git a/internal/health/health_test.go b/internal/health/health_test.go new file mode 100644 index 0000000..2189eba --- /dev/null +++ b/internal/health/health_test.go @@ -0,0 +1,309 @@ +// internal/health/health_test.go +package health_test + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" + "github.com/boneskull/gh-stack/internal/health" +) + +// setupTestRepo creates a temp git repo with an initial commit and returns +// the repo path, a *git.Git, and a *config.Config. The default branch is +// configured as the stack trunk. +func setupTestRepo(t *testing.T) (string, *git.Git, *config.Config) { + t.Helper() + dir := t.TempDir() + + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + run("init") + run("config", "user.email", "test@test.com") + run("config", "user.name", "Test") + + f := filepath.Join(dir, "README.md") + os.WriteFile(f, []byte("# Test"), 0644) + run("add", ".") + run("commit", "-m", "initial") + + g := git.New(dir) + cfg, err := config.New(g) + if err != nil { + t.Fatalf("config.New failed: %v", err) + } + + trunk, err := g.CurrentBranch() + if err != nil { + t.Fatalf("CurrentBranch failed: %v", err) + } + if err := cfg.SetTrunk(trunk); err != nil { + t.Fatalf("SetTrunk failed: %v", err) + } + + return dir, g, cfg +} + +// addTrackedBranch creates a branch off parent with one commit and +// configures stack metadata (parent + fork point). +func addTrackedBranch(t *testing.T, dir string, g *git.Git, cfg *config.Config, branch, parent string) { + t.Helper() + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + // Record fork point before creating branch + parentTip, err := g.GetTip(parent) + if err != nil { + t.Fatalf("GetTip(%s) failed: %v", parent, err) + } + + run("checkout", "-b", branch, parent) + + f := filepath.Join(dir, branch+".txt") + os.WriteFile(f, []byte(branch+" content"), 0644) + run("add", ".") + run("commit", "-m", branch+" commit") + + if err := cfg.SetParent(branch, parent); err != nil { + t.Fatalf("SetParent failed: %v", err) + } + if err := cfg.SetForkPoint(branch, parentTip); err != nil { + t.Fatalf("SetForkPoint failed: %v", err) + } +} + +func TestCheckBranch_Healthy(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + addTrackedBranch(t, dir, g, cfg, "feature", trunk) + + issues := health.CheckBranch(g, cfg, "feature") + if len(issues) != 0 { + t.Errorf("expected no issues, got %v", issues) + } +} + +func TestCheckBranch_MissingBranch(t *testing.T) { + _, g, cfg := setupTestRepo(t) + + // Track a branch that doesn't exist in git + cfg.SetParent("ghost", "main") + + issues := health.CheckBranch(g, cfg, "ghost") + if len(issues) == 0 { + t.Fatal("expected issues for missing branch") + } + if issues[0].Message != "Branch does not exist locally" { + t.Errorf("unexpected message: %s", issues[0].Message) + } + if issues[0].Fixable { + t.Error("missing branch should not be fixable") + } +} + +func TestCheckBranch_MissingParent(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + + // Create a branch tracked against a parent that will be deleted + addTrackedBranch(t, dir, g, cfg, "child", trunk) + + // Now create "middle" as parent, point child at it, then delete it + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + run("checkout", trunk) + run("checkout", "-b", "middle") + run("checkout", trunk) + + cfg.SetParent("child", "middle") + + // Delete the parent branch + run("branch", "-D", "middle") + + issues := health.CheckBranch(g, cfg, "child") + if len(issues) == 0 { + t.Fatal("expected issues for missing parent") + } + if got := issues[0].Message; got != "Parent branch middle does not exist locally" { + t.Errorf("unexpected message: %s", got) + } +} + +func TestCheckBranch_MissingTrunk(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + + addTrackedBranch(t, dir, g, cfg, "feature", trunk) + + // Point the feature's parent at trunk, then rename trunk so it disappears + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + run("checkout", "feature") + run("branch", "-m", trunk, "gone-trunk") + + // feature's parent is still trunk (which no longer exists) + issues := health.CheckBranch(g, cfg, "feature") + if len(issues) == 0 { + t.Fatal("expected issues for missing trunk") + } + found := false + for _, iss := range issues { + if iss.Message == "Trunk branch "+trunk+" does not exist locally" { + found = true + } + } + if !found { + t.Errorf("expected trunk-missing message, got %v", issues) + } +} + +func TestCheckBranch_NoForkPoint(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + + addTrackedBranch(t, dir, g, cfg, "feature", trunk) + + // Remove the fork point from config + cfg.RemoveForkPoint("feature") + + issues := health.CheckBranch(g, cfg, "feature") + if len(issues) == 0 { + t.Fatal("expected issues for missing fork point") + } + if issues[0].Message != "No fork point stored" { + t.Errorf("unexpected message: %s", issues[0].Message) + } + if !issues[0].Fixable { + t.Error("missing fork point should be fixable") + } +} + +func TestCheckBranch_NonexistentForkPointCommit(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + + addTrackedBranch(t, dir, g, cfg, "feature", trunk) + + // Set fork point to a SHA that doesn't exist + fakeSHA := "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + cfg.SetForkPoint("feature", fakeSHA) + + issues := health.CheckBranch(g, cfg, "feature") + if len(issues) == 0 { + t.Fatal("expected issues for nonexistent fork point commit") + } + if !issues[0].Fixable { + t.Error("nonexistent fork point should be fixable") + } +} + +func TestCheckBranch_ForkPointNotAncestor(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + + addTrackedBranch(t, dir, g, cfg, "feature", trunk) + + // Create an orphan commit that exists but isn't an ancestor of feature + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + run("checkout", trunk) + os.WriteFile(filepath.Join(dir, "diverged.txt"), []byte("diverged"), 0644) + run("add", ".") + run("commit", "-m", "diverged commit") + divergedSHA, _ := g.GetTip("HEAD") + + // Set fork point to this diverged commit (exists, but not ancestor of feature) + cfg.SetForkPoint("feature", divergedSHA) + + issues := health.CheckBranch(g, cfg, "feature") + if len(issues) == 0 { + t.Fatal("expected issues for fork point not ancestor") + } + if !issues[0].Fixable { + t.Error("fork point not-ancestor should be fixable") + } +} + +func TestCheckBranch_Drift(t *testing.T) { + dir, g, cfg := setupTestRepo(t) + trunk, _ := g.CurrentBranch() + + addTrackedBranch(t, dir, g, cfg, "feature", trunk) + + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + // Rebase feature onto a new commit on trunk, which moves the actual + // merge-base forward while the stored fork point stays at the old value. + run("checkout", trunk) + os.WriteFile(filepath.Join(dir, "trunk2.txt"), []byte("new trunk"), 0644) + run("add", ".") + run("commit", "-m", "trunk advance") + + run("checkout", "feature") + run("rebase", trunk) + + // Now the stored fork point is the old trunk tip, but merge-base is + // the new trunk tip. That's drift. + issues := health.CheckBranch(g, cfg, "feature") + if len(issues) == 0 { + t.Fatal("expected drift issues") + } + + // The last issue in a drift result should mention "Drift detected" + last := issues[len(issues)-1] + if last.Message != "Drift detected: stored fork point does not match merge-base" { + t.Errorf("expected drift message, got: %s", last.Message) + } + if !last.Fixable { + t.Error("drift should be fixable") + } +} diff --git a/internal/tree/tree_test.go b/internal/tree/tree_test.go index 2c9be2a..7c2c6f1 100644 --- a/internal/tree/tree_test.go +++ b/internal/tree/tree_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/boneskull/gh-stack/internal/config" + "github.com/boneskull/gh-stack/internal/git" "github.com/boneskull/gh-stack/internal/tree" ) @@ -17,7 +18,8 @@ func setupTestRepo(t *testing.T) (*config.Config, string) { exec.Command("git", "-C", dir, "config", "user.email", "test@test.com").Run() exec.Command("git", "-C", dir, "config", "user.name", "Test").Run() - cfg, _ := config.Load(dir) + g := git.New(dir) + cfg, _ := config.New(g) return cfg, dir }