From 06a8591b3d9ef88030768ba9ed51cc56605c081f Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 2 Feb 2026 19:15:09 -0800 Subject: [PATCH 1/6] feat(submit): add interactive PR prompts and adopt existing PRs When creating PRs, users are now prompted to confirm/edit the title and description instead of using auto-generated values silently. The --yes flag skips prompts for non-interactive use. When a PR already exists for a branch (created outside gh-stack), submit now gracefully adopts it instead of failing with HTTP 422. The existing PR's base is updated and stack navigation comments are added. New internal/prompt package provides reusable TTY-aware prompting utilities. --- cmd/submit.go | 152 ++++++++++++++++++++++++-- internal/github/github.go | 22 +--- internal/github/github_test.go | 10 +- internal/prompt/prompt.go | 190 +++++++++++++++++++++++++++++++++ 4 files changed, 342 insertions(+), 32 deletions(-) create mode 100644 internal/prompt/prompt.go diff --git a/cmd/submit.go b/cmd/submit.go index 946e38c..0b78d24 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -9,6 +9,7 @@ import ( "github.com/boneskull/gh-stack/internal/config" "github.com/boneskull/gh-stack/internal/git" "github.com/boneskull/gh-stack/internal/github" + "github.com/boneskull/gh-stack/internal/prompt" "github.com/boneskull/gh-stack/internal/state" "github.com/boneskull/gh-stack/internal/tree" "github.com/spf13/cobra" @@ -33,12 +34,14 @@ var ( submitDryRunFlag bool submitCurrentOnlyFlag bool submitUpdateOnlyFlag bool + submitYesFlag bool ) func init() { submitCmd.Flags().BoolVar(&submitDryRunFlag, "dry-run", false, "show what would be done without doing it") submitCmd.Flags().BoolVar(&submitCurrentOnlyFlag, "current-only", false, "only submit current branch, not descendants") submitCmd.Flags().BoolVar(&submitUpdateOnlyFlag, "update-only", false, "only update existing PRs, don't create new ones") + submitCmd.Flags().BoolVarP(&submitYesFlag, "yes", "y", false, "skip interactive prompts, use generated title/body") rootCmd.AddCommand(submitCmd) } @@ -179,9 +182,12 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr if dryRun { fmt.Printf("Would create PR for %s (base: %s)\n", b.Name, parent) } else { - prNum, err := createPRForBranch(g, ghClient, cfg, root, b.Name, parent, trunk) + prNum, adopted, err := createPRForBranch(g, ghClient, cfg, root, b.Name, parent, trunk) if err != nil { fmt.Printf("Warning: failed to create PR for %s: %v\n", b.Name, err) + } else if adopted { + // adoptExistingPR already printed the message + fmt.Printf(" %s\n", ghClient.PRURL(prNum)) } else { fmt.Printf("Created PR #%d for %s (%s)\n", prNum, b.Name, ghClient.PRURL(prNum)) } @@ -195,26 +201,42 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr } // createPRForBranch creates a PR for the given branch and stores the PR number. -func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string) (int, error) { +// If a PR already exists for the branch, it adopts the existing PR instead. +// Returns (prNumber, adopted, error) where adopted is true if we adopted an existing PR. +func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string) (int, bool, error) { // Determine if draft (not targeting trunk = middle of stack) draft := base != trunk + // Generate default title from branch name + defaultTitle := generateTitleFromBranch(branch) + // Generate PR body from commits - body, bodyErr := generatePRBody(g, base, branch) + defaultBody, bodyErr := generatePRBody(g, base, branch) if bodyErr != nil { // Non-fatal: just skip auto-body fmt.Printf("Warning: could not generate PR body: %v\n", bodyErr) - body = "" + defaultBody = "" } - pr, err := ghClient.CreateSubmitPR(branch, base, body, draft) + // Get title and body (prompt if interactive and --yes not set) + title, body, err := promptForPRDetails(branch, defaultTitle, defaultBody) if err != nil { - return 0, err + return 0, false, fmt.Errorf("failed to get PR details: %w", err) + } + + pr, err := ghClient.CreateSubmitPR(branch, base, title, body, draft) + if err != nil { + // Check if PR already exists - if so, adopt it + if strings.Contains(err.Error(), "pull request already exists") { + prNum, adoptErr := adoptExistingPR(ghClient, cfg, root, branch, base, trunk) + return prNum, true, adoptErr + } + return 0, false, err } // Store PR number in config if err := cfg.SetPR(branch, pr.Number); err != nil { - return pr.Number, fmt.Errorf("PR created but failed to store number: %w", err) + return pr.Number, false, fmt.Errorf("PR created but failed to store number: %w", err) } // Update the tree node's PR number so stack comments render correctly @@ -227,7 +249,121 @@ func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config, fmt.Printf("Warning: failed to add stack comment to PR #%d: %v\n", pr.Number, err) } - return pr.Number, nil + return pr.Number, false, nil +} + +// generateTitleFromBranch creates a PR title from a branch name. +// Replaces - and _ with spaces and converts to title case. +func generateTitleFromBranch(branch string) string { + title := strings.ReplaceAll(branch, "-", " ") + title = strings.ReplaceAll(title, "_", " ") + return toTitleCase(title) +} + +// toTitleCase converts a string to title case (first letter of each word capitalized). +func toTitleCase(s string) string { + words := strings.Fields(s) + for i, word := range words { + if len(word) > 0 { + words[i] = strings.ToUpper(word[:1]) + strings.ToLower(word[1:]) + } + } + return strings.Join(words, " ") +} + +// promptForPRDetails prompts the user for PR title and body. +// If --yes flag is set or stdin is not a TTY, returns the defaults without prompting. +func promptForPRDetails(branch, defaultTitle, defaultBody string) (title, body string, err error) { + // Skip prompts if --yes flag is set + if submitYesFlag { + return defaultTitle, defaultBody, nil + } + + // Skip prompts if not interactive + if !prompt.IsInteractive() { + return defaultTitle, defaultBody, nil + } + + fmt.Printf("\n--- Creating PR for %s ---\n", branch) + + // Prompt for title + title, err = prompt.Input("PR title", defaultTitle) + if err != nil { + return "", "", err + } + + // Show the generated body and ask if user wants to edit + if defaultBody != "" { + fmt.Println("\nGenerated PR description:") + fmt.Println("---") + // Show first few lines or truncate if too long + lines := strings.Split(defaultBody, "\n") + if len(lines) > 10 { + for _, line := range lines[:10] { + fmt.Println(line) + } + fmt.Printf("... (%d more lines)\n", len(lines)-10) + } else { + fmt.Println(defaultBody) + } + fmt.Println("---") + } + + editBody, err := prompt.Confirm("Edit description in editor?", false) + if err != nil { + return "", "", err + } + + if editBody { + body, err = prompt.EditInEditor(defaultBody) + if err != nil { + fmt.Printf("Warning: editor failed, using generated description: %v\n", err) + body = defaultBody + } + } else { + body = defaultBody + } + + fmt.Println() + return title, body, nil +} + +// adoptExistingPR finds an existing PR for the branch and adopts it into the stack. +func adoptExistingPR(ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string) (int, error) { + existingPR, err := ghClient.FindPRByHead(branch) + if err != nil { + return 0, fmt.Errorf("failed to find existing PR: %w", err) + } + if existingPR == nil { + return 0, fmt.Errorf("PR creation failed but no existing PR found for branch %q", branch) + } + + fmt.Printf("Adopting existing PR #%d for %s... ", existingPR.Number, branch) + + // Store PR number in config + if err := cfg.SetPR(branch, existingPR.Number); err != nil { + return existingPR.Number, fmt.Errorf("failed to store PR number: %w", err) + } + + // Update the tree node's PR number so stack comments render correctly + if node := tree.FindNode(root, branch); node != nil { + node.PR = existingPR.Number + } + + // Update PR base to match stack parent + if existingPR.Base.Ref != base { + if err := ghClient.UpdatePRBase(existingPR.Number, base); err != nil { + fmt.Printf("warning: failed to update base: %v\n", err) + } + } + + // Add/update stack navigation comment + if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, existingPR.Number); err != nil { + fmt.Printf("warning: failed to update stack comment: %v\n", err) + } + + fmt.Println("ok") + return existingPR.Number, nil } // generatePRBody creates a PR description from the commits between base and head. diff --git a/internal/github/github.go b/internal/github/github.go index faf5322..39c3db3 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -359,15 +359,10 @@ func (c *Client) UpdateComment(commentID int, body string) error { return nil } -// CreateSubmitPR creates a new pull request with an auto-generated title from the branch name. -// This is a convenience method for the submit workflow. If draft is true, creates a draft PR. +// CreateSubmitPR creates a new pull request. +// This is the primary method for the submit workflow. If draft is true, creates a draft PR. // The body parameter is used as the PR description; pass an empty string for no description. -func (c *Client) CreateSubmitPR(head, base, body string, draft bool) (*PR, error) { - // Generate title from branch name (replace - and _ with spaces, title case) - title := strings.ReplaceAll(head, "-", " ") - title = strings.ReplaceAll(title, "_", " ") - title = toTitleCase(title) - +func (c *Client) CreateSubmitPR(head, base, title, body string, draft bool) (*PR, error) { path := fmt.Sprintf("repos/%s/%s/pulls", c.owner, c.repo) request := struct { @@ -398,17 +393,6 @@ func (c *Client) CreateSubmitPR(head, base, body string, draft bool) (*PR, error return &response, nil } -// toTitleCase converts a string to title case (first letter of each word capitalized). -func toTitleCase(s string) string { - words := strings.Fields(s) - for i, word := range words { - if len(word) > 0 { - words[i] = strings.ToUpper(word[:1]) + strings.ToLower(word[1:]) - } - } - return strings.Join(words, " ") -} - // FindPRByHead finds an open PR with the given head branch. // Returns nil, nil if no PR exists for this branch. func (c *Client) FindPRByHead(branch string) (*PR, error) { diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 363f27d..b3d0e43 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -516,7 +516,7 @@ func TestClient_CreateSubmitPR(t *testing.T) { } client := NewClientWithREST(mock, "owner", "repo") - pr, err := client.CreateSubmitPR("feature-branch", "main", "PR body from commits", false) + pr, err := client.CreateSubmitPR("feature-branch", "main", "Feature Branch", "PR body from commits", false) if err != nil { t.Fatalf("CreateSubmitPR failed: %v", err) @@ -527,9 +527,9 @@ func TestClient_CreateSubmitPR(t *testing.T) { if capturedBody["draft"] != false { t.Errorf("expected draft=false, got %v", capturedBody["draft"]) } - // Title should be auto-generated from branch name - if capturedBody["title"] == "" { - t.Error("expected non-empty title") + // Title should be passed through + if capturedBody["title"] != "Feature Branch" { + t.Errorf("expected title='Feature Branch', got %v", capturedBody["title"]) } // Body should be passed through if capturedBody["body"] != "PR body from commits" { @@ -554,7 +554,7 @@ func TestClient_CreateSubmitPR_Draft(t *testing.T) { } client := NewClientWithREST(mock, "owner", "repo") - pr, err := client.CreateSubmitPR("wip-feature", "develop", "", true) + pr, err := client.CreateSubmitPR("wip-feature", "develop", "WIP Feature", "", true) if err != nil { t.Fatalf("CreateSubmitPR failed: %v", err) diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go new file mode 100644 index 0000000..2370eb4 --- /dev/null +++ b/internal/prompt/prompt.go @@ -0,0 +1,190 @@ +// internal/prompt/prompt.go +package prompt + +import ( + "bufio" + "fmt" + "os" + "os/exec" + "strings" + + "github.com/cli/safeexec" + "github.com/mattn/go-isatty" +) + +// IsInteractive returns true if stdin is connected to a terminal. +func IsInteractive() bool { + return isatty.IsTerminal(os.Stdin.Fd()) || isatty.IsCygwinTerminal(os.Stdin.Fd()) +} + +// Input prompts the user for a single line of input with a default value. +// If the user enters nothing (just presses Enter), the default is returned. +// If stdin is not a TTY, the default is returned without prompting. +func Input(prompt, defaultValue string) (string, error) { + if !IsInteractive() { + return defaultValue, nil + } + + if defaultValue != "" { + fmt.Printf("%s [%s]: ", prompt, defaultValue) + } else { + fmt.Printf("%s: ", prompt) + } + + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return "", fmt.Errorf("failed to read input: %w", err) + } + + input = strings.TrimSpace(input) + if input == "" { + return defaultValue, nil + } + return input, nil +} + +// Confirm prompts the user for a yes/no confirmation. +// Returns the defaultValue if stdin is not a TTY. +func Confirm(prompt string, defaultValue bool) (bool, error) { + if !IsInteractive() { + return defaultValue, nil + } + + defaultStr := "y/N" + if defaultValue { + defaultStr = "Y/n" + } + + fmt.Printf("%s [%s]: ", prompt, defaultStr) + + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return defaultValue, fmt.Errorf("failed to read input: %w", err) + } + + input = strings.TrimSpace(strings.ToLower(input)) + if input == "" { + return defaultValue, nil + } + + switch input { + case "y", "yes": + return true, nil + case "n", "no": + return false, nil + default: + return defaultValue, nil + } +} + +// EditInEditor opens the given text in the user's preferred editor and returns +// the edited content. Uses $VISUAL, then $EDITOR, then falls back to "vi". +// If stdin is not a TTY, returns the original text without editing. +func EditInEditor(text string) (string, error) { + if !IsInteractive() { + return text, nil + } + + // Find editor + editor := os.Getenv("VISUAL") + if editor == "" { + editor = os.Getenv("EDITOR") + } + if editor == "" { + editor = "vi" + } + + // Create temp file + tmpFile, err := os.CreateTemp("", "gh-stack-*.md") + if err != nil { + return "", fmt.Errorf("failed to create temp file: %w", err) + } + tmpPath := tmpFile.Name() + defer os.Remove(tmpPath) //nolint:errcheck // best-effort cleanup + + // Write initial content + if _, writeErr := tmpFile.WriteString(text); writeErr != nil { + tmpFile.Close() //nolint:errcheck // already returning error + return "", fmt.Errorf("failed to write temp file: %w", writeErr) + } + tmpFile.Close() //nolint:errcheck // file written successfully, close errors are ignorable + + // Resolve editor path securely + editorPath, lookupErr := safeexec.LookPath(editor) + if lookupErr != nil { + // Try common editors as fallback + for _, fallback := range []string{"vim", "nano", "vi"} { + if p, e := safeexec.LookPath(fallback); e == nil { + editorPath = p + break + } + } + if editorPath == "" { + return "", fmt.Errorf("no editor found (set $EDITOR): %w", lookupErr) + } + } + + // Run editor + cmd := exec.Command(editorPath, tmpPath) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + if runErr := cmd.Run(); runErr != nil { + return "", fmt.Errorf("editor exited with error: %w", runErr) + } + + // Read edited content + edited, err := os.ReadFile(tmpPath) + if err != nil { + return "", fmt.Errorf("failed to read edited file: %w", err) + } + + return string(edited), nil +} + +// Select prompts the user to choose from a list of options. +// Returns the index of the selected option (0-based). +// If stdin is not a TTY, returns the defaultIndex. +func Select(prompt string, options []string, defaultIndex int) (int, error) { + if !IsInteractive() { + return defaultIndex, nil + } + + fmt.Println(prompt) + for i, opt := range options { + marker := " " + if i == defaultIndex { + marker = "> " + } + fmt.Printf("%s%d. %s\n", marker, i+1, opt) + } + + fmt.Printf("Choice [%d]: ", defaultIndex+1) + + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return defaultIndex, fmt.Errorf("failed to read input: %w", err) + } + + input = strings.TrimSpace(input) + if input == "" { + return defaultIndex, nil + } + + var choice int + if _, err := fmt.Sscanf(input, "%d", &choice); err != nil { + return defaultIndex, nil + } + + // Convert to 0-based index + choice-- + if choice < 0 || choice >= len(options) { + return defaultIndex, nil + } + + return choice, nil +} From a0ee350d4ccf71e6ce1dea4885f5f3ed59b29a3f Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 2 Feb 2026 19:55:15 -0800 Subject: [PATCH 2/6] feat(sync): detect squash-merged branches and prompt for cleanup - Add IsContentMerged() to detect when branch content is in trunk via squash merge (git diff --quiet comparison) - Add interactive prompts for merged branches: delete, orphan, or skip - Handle edge case where user is on the merged branch by checking out trunk first - Change 'mark as ready for review' prompt to default to yes - Update README to reflect actual Go 1.25+ requirement from go.mod --- README.md | 2 +- cmd/sync.go | 145 ++++++++++++++++++++++++++++++------ internal/git/git.go | 44 ++++++++++- internal/git/git_test.go | 157 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 323 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 3da794d..c0b3864 100644 --- a/README.md +++ b/README.md @@ -358,7 +358,7 @@ No remote service required. Your stack relationships stay with your repository. ## Development -To build from source, you'll need Go 1.22+. +To build from source, you'll need Go 1.25+. ```bash gh repo clone boneskull/gh-stack diff --git a/cmd/sync.go b/cmd/sync.go index 33db5f3..bdfa91d 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -4,11 +4,11 @@ package cmd import ( "fmt" "os" - "strings" "github.com/boneskull/gh-stack/internal/config" "github.com/boneskull/gh-stack/internal/git" "github.com/boneskull/gh-stack/internal/github" + "github.com/boneskull/gh-stack/internal/prompt" "github.com/boneskull/gh-stack/internal/tree" "github.com/spf13/cobra" ) @@ -141,6 +141,27 @@ func runSync(cmd *cobra.Command, args []string) error { } } + // Content-based detection for squash merges (fallback when PR detection fails) + // Uses git diff to detect when a branch's content is already in trunk + for _, branch := range branches { + // Skip already detected via PR + if sliceContains(merged, branch) { + continue + } + + // Check if branch content is identical to trunk (squash merge detection) + isContentMerged, diffErr := g.IsContentMerged(branch, trunk) + if diffErr != nil { + // Can't determine, let cascade try + continue + } + + if isContentMerged { + // Tree content is identical - branch was squash-merged + merged = append(merged, branch) + } + } + // Handle merged branches root, _ := tree.Build(cfg) //nolint:errcheck // nil root is fine, FindNode handles it @@ -158,6 +179,17 @@ func runSync(cmd *cobra.Command, args []string) error { continue } + // Handle merged branch with interactive prompt + if syncDryRunFlag { + fmt.Printf("Would handle merged branch %s\n", branch) + } else { + action := handleMergedBranch(g, cfg, branch, trunk, ¤tBranch) + if action == mergedActionSkip { + // User chose to skip - don't collect fork points or retarget children + continue + } + } + // For each child, get fork point - prefer stored, fall back to calculated for _, child := range node.Children { // Try stored fork point first @@ -177,16 +209,6 @@ func runSync(cmd *cobra.Command, args []string) error { childPR: childPR, }) } - - // Now safe to delete the merged branch - if syncDryRunFlag { - fmt.Printf("Would delete merged branch %s\n", branch) - } else { - fmt.Printf("Deleting merged branch %s (PR was merged)\n", branch) - _ = cfg.RemoveParent(branch) //nolint:errcheck // best effort cleanup - _ = cfg.RemovePR(branch) //nolint:errcheck // best effort cleanup - _ = g.DeleteBranch(branch) //nolint:errcheck // best effort cleanup - } } // Retarget children to trunk @@ -205,20 +227,16 @@ func runSync(cmd *cobra.Command, args []string) error { fmt.Printf("Warning: failed to update PR #%d base: %v\n", rt.childPR, updateErr) } - // Check if this was a draft and now targets trunk + // Check if this was a draft and now targets trunk - offer to publish pr, getPRErr := gh.GetPR(rt.childPR) if getPRErr == nil && pr.Draft { fmt.Printf("PR #%d (%s) now targets %s.\n", rt.childPR, rt.childName, trunk) - fmt.Print("Mark as ready for review? [y/N]: ") - - var response string - if _, scanErr := fmt.Scanln(&response); scanErr == nil { - if strings.ToLower(strings.TrimSpace(response)) == "y" { - if readyErr := gh.MarkPRReady(rt.childPR); readyErr != nil { - fmt.Printf("Warning: failed to mark PR ready: %v\n", readyErr) - } else { - fmt.Printf("PR #%d marked as ready for review.\n", rt.childPR) - } + ready, _ := prompt.Confirm("Mark as ready for review?", true) //nolint:errcheck // default is fine + if ready { + if readyErr := gh.MarkPRReady(rt.childPR); readyErr != nil { + fmt.Printf("Warning: failed to mark PR ready: %v\n", readyErr) + } else { + fmt.Printf("PR #%d marked as ready for review.\n", rt.childPR) } } } @@ -281,3 +299,86 @@ func runSync(cmd *cobra.Command, args []string) error { fmt.Println("\nSync complete!") return nil } + +// sliceContains returns true if slice contains the given string. +func sliceContains(slice []string, s string) bool { + for _, item := range slice { + if item == s { + return true + } + } + return false +} + +// mergedAction represents the user's choice for handling a merged branch. +type mergedAction int + +const ( + mergedActionDelete mergedAction = iota + mergedActionOrphan + mergedActionSkip +) + +// handleMergedBranch prompts the user for how to handle a merged branch and executes the choice. +// Returns the action taken. If the user is on the merged branch, it will checkout trunk first. +// The currentBranch pointer is updated if a checkout occurs. +func handleMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, currentBranch *string) mergedAction { + fmt.Printf("\nBranch %q appears to be merged into %s.\n", branch, trunk) + + // Default to delete in non-interactive mode + if !prompt.IsInteractive() { + return deleteMergedBranch(g, cfg, branch, trunk, currentBranch) + } + + // Interactive mode: prompt for action + choice, _ := prompt.Select("What would you like to do?", []string{ //nolint:errcheck // default is fine on error + "Delete branch and remove from stack", + "Orphan (keep branch, remove from stack)", + "Skip (keep in stack, may cause conflicts)", + }, 0) + + switch choice { + case 0: // Delete + return deleteMergedBranch(g, cfg, branch, trunk, currentBranch) + case 1: // Orphan + return orphanMergedBranch(cfg, branch) + case 2: // Skip + fmt.Printf("Skipping %s (keeping in stack)\n", branch) + return mergedActionSkip + default: + return deleteMergedBranch(g, cfg, branch, trunk, currentBranch) + } +} + +// deleteMergedBranch deletes a merged branch and removes it from stack config. +// If the user is on the branch, it checks out trunk first. +func deleteMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, currentBranch *string) mergedAction { + // If user is on the merged branch, checkout trunk first + if *currentBranch == branch { + fmt.Printf("Checking out %s (currently on merged branch)...\n", trunk) + if err := g.Checkout(trunk); err != nil { + fmt.Printf("Warning: could not checkout %s: %v\n", trunk, err) + fmt.Printf("Falling back to orphan instead of delete.\n") + return orphanMergedBranch(cfg, branch) + } + *currentBranch = trunk + } + + fmt.Printf("Deleting merged branch %s\n", branch) + _ = cfg.RemoveParent(branch) //nolint:errcheck // best effort cleanup + _ = cfg.RemovePR(branch) //nolint:errcheck // best effort cleanup + _ = cfg.RemoveForkPoint(branch) //nolint:errcheck // best effort cleanup + if err := g.DeleteBranch(branch); err != nil { + fmt.Printf("Warning: could not delete branch %s: %v\n", branch, err) + } + return mergedActionDelete +} + +// orphanMergedBranch removes a branch from stack config but keeps the git branch. +func orphanMergedBranch(cfg *config.Config, branch string) mergedAction { + fmt.Printf("Orphaning %s (branch preserved, removed from stack)\n", branch) + _ = cfg.RemoveParent(branch) //nolint:errcheck // best effort cleanup + _ = cfg.RemovePR(branch) //nolint:errcheck // best effort cleanup + _ = cfg.RemoveForkPoint(branch) //nolint:errcheck // best effort cleanup + return mergedActionOrphan +} diff --git a/internal/git/git.go b/internal/git/git.go index 27d2ca0..9f0edd2 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -178,6 +178,47 @@ func (g *Git) CommitExists(sha string) bool { return err == nil } +// HasUnmergedCommits returns true if the branch has commits not yet in upstream. +// Uses git cherry to detect by diff content, which works for cherry-picks +// where the commit SHAs differ but the content is the same. +// Note: This does NOT detect squash merges where multiple commits are combined. +// Use IsContentMerged for squash merge detection. +func (g *Git) HasUnmergedCommits(branch, upstream string) (bool, error) { + // git cherry -v upstream branch + // Returns lines prefixed with + (unmerged) or - (merged/equivalent) + out, err := g.run("cherry", "-v", upstream, branch) + if err != nil { + return false, err + } + + // Empty output means no commits unique to branch + if out == "" { + return false, nil + } + + // If any line starts with +, there are unmerged commits + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(line, "+ ") { + return true, nil + } + } + return false, nil +} + +// IsContentMerged returns true if the branch has no content differences from upstream. +// This detects squash merges where the tree content is identical even though +// the commit history differs. Returns true if `git diff upstream branch` is empty. +func (g *Git) IsContentMerged(branch, upstream string) (bool, error) { + // git diff upstream branch --quiet exits 0 if no diff, 1 if diff exists + err := g.runSilent("diff", "--quiet", upstream, branch) + if err != nil { + // Exit code 1 means there are differences + return false, nil + } + // Exit code 0 means no differences - content is merged + return true, nil +} + // RebaseOnto rebases a branch onto a new base, replaying only commits after oldBase. // Checks out the branch first, then runs: git rebase --onto // Useful when a parent branch was squash-merged and we need to replay only @@ -257,8 +298,7 @@ func (g *Git) GetCommits(base, head string) ([]Commit, error) { var commits []Commit // Split by double null (between commits) - entries := strings.Split(out, "\x00\x00") - for _, entry := range entries { + for entry := range strings.SplitSeq(out, "\x00\x00") { entry = strings.TrimSpace(entry) if entry == "" { continue diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 18911bd..cc1f4e6 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -438,3 +438,160 @@ func TestRebaseOnto(t *testing.T) { t.Error("parent.txt should NOT exist - only child's commits should be replayed") } } + +func TestHasUnmergedCommits(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create feature branch with a commit + g.CreateAndCheckout("feature") + os.WriteFile(filepath.Join(dir, "feature.txt"), []byte("feature content"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "feature commit").Run() + + // Feature should have unmerged commits relative to trunk + hasUnmerged, err := g.HasUnmergedCommits("feature", trunk) + if err != nil { + t.Fatalf("HasUnmergedCommits failed: %v", err) + } + if !hasUnmerged { + t.Error("feature should have unmerged commits") + } +} + +func TestHasUnmergedCommitsNone(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create feature branch at same commit as trunk (no new commits) + g.CreateBranch("feature") + + // Feature should have no unmerged commits + hasUnmerged, err := g.HasUnmergedCommits("feature", trunk) + if err != nil { + t.Fatalf("HasUnmergedCommits failed: %v", err) + } + if hasUnmerged { + t.Error("feature should have no unmerged commits when at same commit as trunk") + } +} + +func TestHasUnmergedCommitsSquashMerged(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create feature branch with a commit + g.CreateAndCheckout("feature") + os.WriteFile(filepath.Join(dir, "feature.txt"), []byte("feature content"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "feature commit").Run() + + // Simulate squash merge: go back to trunk and create a commit with the same content + g.Checkout(trunk) + os.WriteFile(filepath.Join(dir, "feature.txt"), []byte("feature content"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "squash merged: feature commit").Run() + + // Feature should now have no unmerged commits (content is equivalent) + hasUnmerged, err := g.HasUnmergedCommits("feature", trunk) + if err != nil { + t.Fatalf("HasUnmergedCommits failed: %v", err) + } + if hasUnmerged { + t.Error("feature should have no unmerged commits after squash merge (content equivalent)") + } +} + +func TestHasUnmergedCommitsPartialMerge(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create feature branch with two commits + g.CreateAndCheckout("feature") + + os.WriteFile(filepath.Join(dir, "file1.txt"), []byte("content1"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "first commit").Run() + + os.WriteFile(filepath.Join(dir, "file2.txt"), []byte("content2"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "second commit").Run() + + // Simulate partial squash merge: only the first commit's content + g.Checkout(trunk) + os.WriteFile(filepath.Join(dir, "file1.txt"), []byte("content1"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "squash: first commit").Run() + + // Feature should still have unmerged commits (second commit not merged) + hasUnmerged, err := g.HasUnmergedCommits("feature", trunk) + if err != nil { + t.Fatalf("HasUnmergedCommits failed: %v", err) + } + if !hasUnmerged { + t.Error("feature should have unmerged commits (second commit not merged)") + } +} + +func TestIsContentMerged(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create feature branch with a commit + g.CreateAndCheckout("feature") + os.WriteFile(filepath.Join(dir, "feature.txt"), []byte("feature content"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "feature commit").Run() + + // Feature has different content than trunk + merged, err := g.IsContentMerged("feature", trunk) + if err != nil { + t.Fatalf("IsContentMerged failed: %v", err) + } + if merged { + t.Error("feature should not be content-merged (different content)") + } +} + +func TestIsContentMergedSquash(t *testing.T) { + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create feature branch with multiple commits + g.CreateAndCheckout("feature") + os.WriteFile(filepath.Join(dir, "file1.txt"), []byte("content1"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "first commit").Run() + + os.WriteFile(filepath.Join(dir, "file2.txt"), []byte("content2"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "second commit").Run() + + // Simulate squash merge: trunk gets all the content in one commit + g.Checkout(trunk) + os.WriteFile(filepath.Join(dir, "file1.txt"), []byte("content1"), 0644) + os.WriteFile(filepath.Join(dir, "file2.txt"), []byte("content2"), 0644) + exec.Command("git", "-C", dir, "add", ".").Run() + exec.Command("git", "-C", dir, "commit", "-m", "squash: feature").Run() + + // Feature content should now be merged (identical trees) + merged, err := g.IsContentMerged("feature", trunk) + if err != nil { + t.Fatalf("IsContentMerged failed: %v", err) + } + if !merged { + t.Error("feature should be content-merged (identical content after squash)") + } +} From 3582fa5060b30fe6a3cc844776b3d0adc4141928 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 2 Feb 2026 20:07:13 -0800 Subject: [PATCH 3/6] fix: improve error message for untracked branches Show actionable guidance when submit or cascade is run on an untracked branch, suggesting the 'gh stack adopt' command with examples. --- cmd/cascade.go | 3 ++- cmd/submit.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/cascade.go b/cmd/cascade.go index eae4aa3..5c73127 100644 --- a/cmd/cascade.go +++ b/cmd/cascade.go @@ -74,7 +74,8 @@ func runCascade(cmd *cobra.Command, args []string) error { node := tree.FindNode(root, currentBranch) if node == nil { - return fmt.Errorf("branch %q is not tracked", currentBranch) + trunk, _ := cfg.GetTrunk() //nolint:errcheck // empty is fine for error message + return fmt.Errorf("branch %q is not tracked in the stack\n\nTo add it, run:\n gh stack adopt %s # to stack on %s\n gh stack adopt -p # to stack on a different branch", currentBranch, trunk, trunk) } // Collect branches to cascade diff --git a/cmd/submit.go b/cmd/submit.go index 0b78d24..c9d1fe3 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -85,7 +85,8 @@ func runSubmit(cmd *cobra.Command, args []string) error { node := tree.FindNode(root, currentBranch) if node == nil { - return fmt.Errorf("branch %q is not tracked", currentBranch) + trunk, _ := cfg.GetTrunk() //nolint:errcheck // empty is fine for error message + return fmt.Errorf("branch %q is not tracked in the stack\n\nTo add it, run:\n gh stack adopt %s # to stack on %s\n gh stack adopt -p # to stack on a different branch", currentBranch, trunk, trunk) } // Collect branches to submit (current + descendants) From 4c3d7b148e475b78515108abb033eda113fc8c42 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 2 Feb 2026 20:26:07 -0800 Subject: [PATCH 4/6] fix: clear macOS extended attributes in gh-install The com.apple.provenance extended attribute causes the binary to hang when run as a gh extension. Clear all extended attributes after copying. --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 004d0fd..56d0d47 100644 --- a/Makefile +++ b/Makefile @@ -31,6 +31,8 @@ clean: gh-install: build mkdir -p ~/.local/share/gh/extensions/gh-stack cp gh-stack ~/.local/share/gh/extensions/gh-stack/ + @# Clear macOS extended attributes that can cause hangs + @xattr -c ~/.local/share/gh/extensions/gh-stack/gh-stack 2>/dev/null || true # Install development tools tools: From 1768f984cf024987a582037dd01a89ffdafe4ded Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 2 Feb 2026 20:51:09 -0800 Subject: [PATCH 5/6] feat(sync): detect parent branches missing from remote When a branch's parent exists locally but not on the remote, sync now detects this and prompts to retarget to trunk. This handles cases where a parent branch was deleted without merging, or never pushed. - Add RemoteBranchExists() to check if branch exists on origin - Add detection in sync after merged branch handling - Add unit test for RemoteBranchExists --- cmd/sync.go | 43 ++++++++++++++++++++++++++++++++++++++++ internal/git/git.go | 6 ++++++ internal/git/git_test.go | 40 +++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/cmd/sync.go b/cmd/sync.go index bdfa91d..3292f2c 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -162,6 +162,49 @@ func runSync(cmd *cobra.Command, args []string) error { } } + // Check for branches whose parent doesn't exist on the remote + // This can happen if a parent branch was deleted without merging, or never pushed + for _, branch := range branches { + parent, parentErr := cfg.GetParent(branch) + if parentErr != nil { + continue + } + + // Skip if parent is trunk (trunk should always exist on remote) + if parent == trunk { + continue + } + + // Skip if parent is already marked as merged (will be handled) + if sliceContains(merged, parent) { + continue + } + + // Check if parent exists on remote + if !g.RemoteBranchExists(parent) { + fmt.Printf("\nWarning: parent branch %q of %q does not exist on remote.\n", parent, branch) + if prompt.IsInteractive() { + retarget, _ := prompt.Confirm(fmt.Sprintf("Retarget %s to %s?", branch, trunk), true) //nolint:errcheck // default is fine + if retarget { + _ = cfg.SetParent(branch, trunk) //nolint:errcheck // best effort + fmt.Printf("Retargeted %s to %s\n", branch, trunk) + + // Update PR base on GitHub if PR exists + prNum, _ := cfg.GetPR(branch) //nolint:errcheck // 0 is fine + if prNum > 0 { + if updateErr := gh.UpdatePRBase(prNum, trunk); updateErr != nil { + fmt.Printf("Warning: failed to update PR #%d base: %v\n", prNum, updateErr) + } else { + fmt.Printf("Updated PR #%d base to %s\n", prNum, trunk) + } + } + } + } else { + fmt.Printf("Run 'git config branch.%s.stackParent %s' to fix.\n", branch, trunk) + } + } + } + // Handle merged branches root, _ := tree.Build(cfg) //nolint:errcheck // nil root is fine, FindNode handles it diff --git a/internal/git/git.go b/internal/git/git.go index 9f0edd2..d56ebdb 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -219,6 +219,12 @@ func (g *Git) IsContentMerged(branch, upstream string) (bool, error) { return true, nil } +// RemoteBranchExists checks if a branch exists on the remote (origin). +func (g *Git) RemoteBranchExists(branch string) bool { + err := g.runSilent("ls-remote", "--exit-code", "--heads", "origin", branch) + return err == nil +} + // RebaseOnto rebases a branch onto a new base, replaying only commits after oldBase. // Checks out the branch first, then runs: git rebase --onto // Useful when a parent branch was squash-merged and we need to replay only diff --git a/internal/git/git_test.go b/internal/git/git_test.go index cc1f4e6..ec47de1 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -595,3 +595,43 @@ func TestIsContentMergedSquash(t *testing.T) { t.Error("feature should be content-merged (identical content after squash)") } } + +func TestRemoteBranchExists(t *testing.T) { + // Create main repo + dir := setupTestRepo(t) + g := git.New(dir) + + trunk, _ := g.CurrentBranch() + + // Create a bare repo as "remote" + remoteDir := t.TempDir() + exec.Command("git", "clone", "--bare", dir, remoteDir).Run() + exec.Command("git", "-C", dir, "remote", "add", "origin", remoteDir).Run() + exec.Command("git", "-C", dir, "push", "-u", "origin", trunk).Run() + + // Trunk should exist on remote + if !g.RemoteBranchExists(trunk) { + t.Errorf("RemoteBranchExists(%s) = false, want true", trunk) + } + + // Create a local-only branch (not pushed) + g.CreateBranch("local-only") + + // Local-only branch should NOT exist on remote + if g.RemoteBranchExists("local-only") { + t.Error("RemoteBranchExists(local-only) = true, want false (not pushed)") + } + + // Push the branch + exec.Command("git", "-C", dir, "push", "origin", "local-only").Run() + + // Now it should exist + if !g.RemoteBranchExists("local-only") { + t.Error("RemoteBranchExists(local-only) = false, want true (after push)") + } + + // Non-existent branch should return false + if g.RemoteBranchExists("nonexistent-branch-xyz") { + t.Error("RemoteBranchExists(nonexistent) = true, want false") + } +} From 122c96d5b96365bcd4ae0b6631c6132cfac16cff Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 2 Feb 2026 21:00:57 -0800 Subject: [PATCH 6/6] fix: address PR review comments - Improve --yes flag help text to clarify it affects PR creation - Unify adopted PR message format with created PR format - Fix warning capitalization consistency (Warning: not warning:) - Add --yes hint in PR creation prompt - Validate PR title is not empty - Trim trailing whitespace in EditInEditor - Add bounds validation for Select defaultIndex --- cmd/submit.go | 18 +++++++++--------- internal/prompt/prompt.go | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/cmd/submit.go b/cmd/submit.go index c9d1fe3..843664c 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -41,7 +41,7 @@ func init() { submitCmd.Flags().BoolVar(&submitDryRunFlag, "dry-run", false, "show what would be done without doing it") submitCmd.Flags().BoolVar(&submitCurrentOnlyFlag, "current-only", false, "only submit current branch, not descendants") submitCmd.Flags().BoolVar(&submitUpdateOnlyFlag, "update-only", false, "only update existing PRs, don't create new ones") - submitCmd.Flags().BoolVarP(&submitYesFlag, "yes", "y", false, "skip interactive prompts, use generated title/body") + submitCmd.Flags().BoolVarP(&submitYesFlag, "yes", "y", false, "skip interactive prompts and use auto-generated title/description for PRs") rootCmd.AddCommand(submitCmd) } @@ -187,8 +187,7 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr if err != nil { fmt.Printf("Warning: failed to create PR for %s: %v\n", b.Name, err) } else if adopted { - // adoptExistingPR already printed the message - fmt.Printf(" %s\n", ghClient.PRURL(prNum)) + fmt.Printf("Adopted PR #%d for %s (%s)\n", prNum, b.Name, ghClient.PRURL(prNum)) } else { fmt.Printf("Created PR #%d for %s (%s)\n", prNum, b.Name, ghClient.PRURL(prNum)) } @@ -285,13 +284,17 @@ func promptForPRDetails(branch, defaultTitle, defaultBody string) (title, body s return defaultTitle, defaultBody, nil } - fmt.Printf("\n--- Creating PR for %s ---\n", branch) + fmt.Printf("\n--- Creating PR for %s (use --yes to skip prompts) ---\n", branch) // Prompt for title title, err = prompt.Input("PR title", defaultTitle) if err != nil { return "", "", err } + title = strings.TrimSpace(title) + if title == "" { + return "", "", fmt.Errorf("PR title cannot be empty") + } // Show the generated body and ask if user wants to edit if defaultBody != "" { @@ -339,8 +342,6 @@ func adoptExistingPR(ghClient *github.Client, cfg *config.Config, root *tree.Nod return 0, fmt.Errorf("PR creation failed but no existing PR found for branch %q", branch) } - fmt.Printf("Adopting existing PR #%d for %s... ", existingPR.Number, branch) - // Store PR number in config if err := cfg.SetPR(branch, existingPR.Number); err != nil { return existingPR.Number, fmt.Errorf("failed to store PR number: %w", err) @@ -354,16 +355,15 @@ func adoptExistingPR(ghClient *github.Client, cfg *config.Config, root *tree.Nod // Update PR base to match stack parent if existingPR.Base.Ref != base { if err := ghClient.UpdatePRBase(existingPR.Number, base); err != nil { - fmt.Printf("warning: failed to update base: %v\n", err) + fmt.Printf("Warning: failed to update base: %v\n", err) } } // Add/update stack navigation comment if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, existingPR.Number); err != nil { - fmt.Printf("warning: failed to update stack comment: %v\n", err) + fmt.Printf("Warning: failed to update stack comment: %v\n", err) } - fmt.Println("ok") return existingPR.Number, nil } diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index 2370eb4..606ec54 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -142,13 +142,26 @@ func EditInEditor(text string) (string, error) { return "", fmt.Errorf("failed to read edited file: %w", err) } - return string(edited), nil + // Normalize line endings and trim trailing whitespace + result := strings.ReplaceAll(string(edited), "\r\n", "\n") + result = strings.TrimRight(result, "\n\t ") + + return result, nil } // Select prompts the user to choose from a list of options. // Returns the index of the selected option (0-based). // If stdin is not a TTY, returns the defaultIndex. func Select(prompt string, options []string, defaultIndex int) (int, error) { + if len(options) == 0 { + return 0, fmt.Errorf("no options provided") + } + + // Clamp defaultIndex to valid range + if defaultIndex < 0 || defaultIndex >= len(options) { + defaultIndex = 0 + } + if !IsInteractive() { return defaultIndex, nil }