diff --git a/.github/actions/claude-code-review/action.yml b/.github/actions/claude-code-review/action.yml index 956a128..9efc91f 100644 --- a/.github/actions/claude-code-review/action.yml +++ b/.github/actions/claude-code-review/action.yml @@ -16,7 +16,7 @@ runs: using: composite steps: - name: Harden-Runner - uses: step-security/harden-runner@f808768d1510423e83855289c910610ca9b43176 # v2.17.0 + uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0 with: egress-policy: audit @@ -30,7 +30,7 @@ runs: # Pinned to v1 for supply-chain safety uses: anthropics/claude-code-action@b2fdd80112e5f140e097b11d7a3d9edf0b226fd0 with: - allowed_bots: 'renovate[bot]' + allowed_bots: 'renovate[bot],cursor' claude_args: | --max-turns 20 track_progress: true diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 283a561..e759317 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -12,6 +12,10 @@ on: jobs: claude: + # NOTE: For issues:edited, author_association refers to the issue author, not + # the actor who performed the edit. This means an edit by a non-collaborator + # could re-trigger Claude if the original author is a collaborator. In practice, + # only users with write access can edit others' issues, so the risk is limited. if: | (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') && @@ -25,10 +29,6 @@ jobs: (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')) && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.issue.author_association)) - # NOTE: For issues:edited, author_association refers to the issue author, not - # the actor who performed the edit. This means an edit by a non-collaborator - # could re-trigger Claude if the original author is a collaborator. In practice, - # only users with write access can edit others' issues, so the risk is limited. concurrency: group: claude-${{ github.repository }}-${{ github.event.issue.number || github.event.pull_request.number || github.run_id }} cancel-in-progress: true @@ -41,7 +41,7 @@ jobs: actions: read # Required for Claude to read CI results on PRs steps: - name: Harden-Runner - uses: step-security/harden-runner@f808768d1510423e83855289c910610ca9b43176 # v2.17.0 + uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0 with: egress-policy: audit diff --git a/.release-please-manifest.json b/.release-please-manifest.json index bf5f1fb..37fcefa 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "0.6.2" + ".": "1.0.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d39814..da9848e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,30 @@ # Changelog +## [1.0.0](https://github.com/boneskull/gh-stack/compare/v0.6.3...v1.0.0) (2026-04-20) + + +### ⚠ BREAKING CHANGES + +* Some flags have changed. `restack`: `--only` is now `-c, --current`; `submit`: `--current-only` is now `-c, --current`, `--update-only` is now `-u, --update`, `--push-only` is now `-s, --skip-prs`, removed `-w` shorthand from `--web`. + +### Bug Fixes + +* normalize flags ([#97](https://github.com/boneskull/gh-stack/issues/97)) ([91a896d](https://github.com/boneskull/gh-stack/commit/91a896d092afb5a90147bc10339d05b1ec4aa597)) +* **submit:** only prompt to publish draft when base transitions to trunk ([#100](https://github.com/boneskull/gh-stack/issues/100)) ([c47ca16](https://github.com/boneskull/gh-stack/commit/c47ca167d25b1ed0235e2a376a7a0efed7dd871e)) + +## [0.6.3](https://github.com/boneskull/gh-stack/compare/v0.6.2...v0.6.3) (2026-04-18) + + +### Bug Fixes + +* **cmd:** defer submit push until PR intent is known ([96fb5b9](https://github.com/boneskull/gh-stack/commit/96fb5b9e981137fa2ab8bc8f21afca670ca86bf6)) +* **cmd:** only push stack branches when submit will touch PRs ([#89](https://github.com/boneskull/gh-stack/issues/89)) ([96fb5b9](https://github.com/boneskull/gh-stack/commit/96fb5b9e981137fa2ab8bc8f21afca670ca86bf6)) +* **deps:** update module github.com/charmbracelet/bubbles to v0.21.1 ([#87](https://github.com/boneskull/gh-stack/issues/87)) ([37e572a](https://github.com/boneskull/gh-stack/commit/37e572a5f513c5379fa565eb3e424190b6d01332)) +* **prompt:** support ESC to skip PR creation ([#85](https://github.com/boneskull/gh-stack/issues/85)) ([bfce25d](https://github.com/boneskull/gh-stack/commit/bfce25d3d04ed6201a40344a7d25a5425bb38765)) +* **prompt:** support ESC to skip PR creation in huh form ([bfce25d](https://github.com/boneskull/gh-stack/commit/bfce25d3d04ed6201a40344a7d25a5425bb38765)), closes [#78](https://github.com/boneskull/gh-stack/issues/78) +* rename cascade to restack more generally ([#94](https://github.com/boneskull/gh-stack/issues/94)) ([115ce65](https://github.com/boneskull/gh-stack/commit/115ce650a8621eca5c070b1a034b0fa8788085e4)) +* **restack:** prevent stale fork points from triggering incorrect --onto rebases ([#83](https://github.com/boneskull/gh-stack/issues/83)) ([a9af01a](https://github.com/boneskull/gh-stack/commit/a9af01a1e7f39896023bd5d7cecfbcb2a9744668)) + ## [0.6.2](https://github.com/boneskull/gh-stack/compare/v0.6.1...v0.6.2) (2026-04-08) diff --git a/README.md b/README.md index 99b15a4..77ed874 100644 --- a/README.md +++ b/README.md @@ -170,10 +170,10 @@ Display the branch tree showing the stack hierarchy, current branch, and associa #### log Flags -| Flag | Description | -| ------------- | ------------------------------------- | -| `--all` | Show all branches | -| `--porcelain` | Machine-readable tab-separated output | +| Flag | Description | +| ----------------- | ------------------------------------- | +| `-a, --all` | Show all branches | +| `-p, --porcelain` | Machine-readable tab-separated output | #### Porcelain Format @@ -197,10 +197,10 @@ gh stack create #### create Flags -| Flag | Description | -| --------------- | ----------------------------------------------- | -| `-m, --message` | Commit message for staged changes | -| `--empty` | Create branch without committing staged changes | +| Flag | Description | +| ----------------- | ----------------------------------------------- | +| `-m, --message` | Commit message for staged changes | +| `-C, --no-commit` | Create branch without committing staged changes | ### adopt @@ -216,9 +216,9 @@ gh stack adopt #### adopt Flags -| Flag | Description | -| ---------- | ----------------------------------------- | -| `--branch` | Branch to adopt (default: current branch) | +| Flag | Description | +| -------------- | ----------------------------------------- | +| `-b, --branch` | Branch to adopt (default: current branch) | ### orphan @@ -236,9 +236,9 @@ If no branch is specified, orphans the current branch. #### orphan Flags -| Flag | Description | -| --------- | --------------------------- | -| `--force` | Also orphan all descendants | +| Flag | Description | +| ------------- | --------------------------- | +| `-f, --force` | Also orphan all descendants | ### link @@ -276,15 +276,15 @@ If a rebase conflict occurs, resolve it and run `gh stack continue`. #### submit Flags -| Flag | Description | -| ---------------- | ------------------------------------------------------------------------ | -| `--dry-run` | Show what would happen without doing it | -| `--from [branch]` | Submit from this branch toward leaves (bare `--from` = current branch) | -| `--current-only` | Only submit the current branch, not descendants | -| `--update-only` | Only update existing PRs, don't create new ones | -| `--push-only` | Skip PR creation/update, only restack and push | -| `-y, --yes` | Skip interactive prompts; use auto-generated PR title/description | -| `-w, --web` | Open created/updated PRs in web browser | +| Flag | Description | +| --------------------- | ---------------------------------------------------------------------- | +| `-D, --dry-run` | Show what would happen without doing it | +| `-f, --from [branch]` | Submit from this branch toward leaves (bare `--from` = current branch) | +| `-c, --current` | Only submit the current branch, not descendants | +| `-u, --update` | Only update existing PRs, don't create new ones | +| `-s, --skip-prs` | Skip PR creation/update, only restack and push | +| `-y, --yes` | Skip interactive prompts; use auto-generated PR title/description | +| `--web` | Open created/updated PRs in web browser | ### restack @@ -296,11 +296,11 @@ If a rebase conflict occurs, resolve it and run `gh stack continue`. #### restack Flags -| Flag | Description | -| ------------- | -------------------------------------------------------- | -| `--only` | Only restack current branch, not descendants | -| `--dry-run` | Show what would be done | -| `--worktrees` | Rebase branches checked out in linked worktrees in-place | +| Flag | Description | +| ----------------- | -------------------------------------------------------- | +| `-c, --current` | Only restack current branch, not descendants | +| `-D, --dry-run` | Show what would be done | +| `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | ### continue @@ -322,11 +322,11 @@ This is the command to run when upstream changes have occurred (e.g., a PR in yo #### sync Flags -| Flag | Description | -| -------------- | -------------------------------------------------------- | -| `--no-restack` | Skip restacking branches | -| `--dry-run` | Show what would be done | -| `--worktrees` | Rebase branches checked out in linked worktrees in-place | +| Flag | Description | +| ----------------- | -------------------------------------------------------- | +| `--no-restack` | Skip restacking branches (might not work well!) | +| `-D, --dry-run` | Show what would be done | +| `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | ### undo @@ -346,10 +346,10 @@ Snapshots are stored in `.git/stack-undo/` and archived to `.git/stack-undo/done #### undo Flags -| Flag | Description | -| ----------- | -------------------------------------------- | -| `--force` | Skip confirmation prompt | -| `--dry-run` | Show what would be restored without doing it | +| Flag | Description | +| --------------- | -------------------------------------------- | +| `-f, --force` | Skip confirmation prompt | +| `-D, --dry-run` | Show what would be restored without doing it | ## Working with Git Worktrees diff --git a/cmd/abort.go b/cmd/abort.go index 40f5bc3..a2b79bb 100644 --- a/cmd/abort.go +++ b/cmd/abort.go @@ -33,7 +33,7 @@ func runAbort(cmd *cobra.Command, args []string) error { g := git.New(cwd) - // Check if cascade in progress + // Check if restack in progress st, err := state.Load(g.GetGitDir()) if err != nil { return errors.New("no operation in progress") diff --git a/cmd/adopt.go b/cmd/adopt.go index da8c49d..2bfa924 100644 --- a/cmd/adopt.go +++ b/cmd/adopt.go @@ -26,7 +26,7 @@ By default, adopts the current branch. Use --branch to specify a different branc var adoptBranchFlag string func init() { - adoptCmd.Flags().StringVar(&adoptBranchFlag, "branch", "", "branch to adopt (default: current branch)") + adoptCmd.Flags().StringVarP(&adoptBranchFlag, "branch", "b", "", "branch to adopt (default: current branch)") rootCmd.AddCommand(adoptCmd) } diff --git a/cmd/continue.go b/cmd/continue.go index 9dacf20..fc6b052 100644 --- a/cmd/continue.go +++ b/cmd/continue.go @@ -69,7 +69,7 @@ func runContinue(cmd *cobra.Command, args []string) error { } // Update fork point for the branch that just finished rebasing. - // The cascade loop's fork point update only fires after a non-conflicting + // The restack loop's fork point update only fires after a non-conflicting // rebase, so branches that hit a conflict never get their fork point // refreshed -- leaving it stale for future operations. if parent, parentErr := cfg.GetParent(st.Current); parentErr == nil { @@ -84,7 +84,7 @@ func runContinue(cmd *cobra.Command, args []string) error { return err } - // If there are more branches to cascade, continue cascading + // If there are more branches to restack, continue cascading if len(st.Pending) > 0 { var branches []*tree.Node for _, name := range st.Pending { @@ -96,7 +96,7 @@ func runContinue(cmd *cobra.Command, args []string) error { // Remove state file before continuing (will be recreated if conflict) _ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup - if cascadeErr := doCascadeWithState(g, cfg, branches, CascadeOptions{ + if restackErr := doRestackWithState(g, cfg, branches, RestackOptions{ Operation: st.Operation, UpdateOnly: st.UpdateOnly, OpenWeb: st.Web, @@ -104,18 +104,18 @@ func runContinue(cmd *cobra.Command, args []string) error { Branches: st.Branches, StashRef: st.StashRef, Worktrees: st.Worktrees, - }, s); cascadeErr != nil { - // Stash handling is done by doCascadeWithState (conflict saves in state, errors restore) - if !errors.Is(cascadeErr, ErrConflict) && st.StashRef != "" { + }, s); restackErr != nil { + // Stash handling is done by doRestackWithState (conflict saves in state, errors restore) + if !errors.Is(restackErr, ErrConflict) && st.StashRef != "" { fmt.Println("Restoring auto-stashed changes...") if popErr := g.StashPop(st.StashRef); popErr != nil { fmt.Printf("%s could not restore stashed changes (commit %s): %v\n", s.WarningIcon(), git.AbbrevSHA(st.StashRef), popErr) } } - return cascadeErr // Another conflict - state saved + return restackErr // Another conflict - state saved } } else { - // No more branches to cascade - cleanup state + // No more branches to restack - cleanup state _ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup } @@ -160,7 +160,7 @@ func runContinue(cmd *cobra.Command, args []string) error { return err } - // Restore stash after cascade completes + // Restore stash after restack completes if st.StashRef != "" { fmt.Println("Restoring auto-stashed changes...") if popErr := g.StashPop(st.StashRef); popErr != nil { diff --git a/cmd/create.go b/cmd/create.go index fa2aa58..353d554 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -27,7 +27,7 @@ var ( func init() { createCmd.Flags().StringVarP(&createMessageFlag, "message", "m", "", "commit message for staged changes") - createCmd.Flags().BoolVar(&createEmptyFlag, "empty", false, "create branch without committing staged changes") + createCmd.Flags().BoolVarP(&createEmptyFlag, "no-commit", "C", false, "create branch without committing staged changes") rootCmd.AddCommand(createCmd) } @@ -77,7 +77,7 @@ func runCreate(cmd *cobra.Command, args []string) error { if hasStaged && !createEmptyFlag { if createMessageFlag == "" { - return errors.New("staged changes found but no commit message provided; use -m or --empty") + return errors.New("staged changes found but no commit message provided; use -m or --no-commit") } } diff --git a/cmd/create_test.go b/cmd/create_test.go index ba23992..0a124fe 100644 --- a/cmd/create_test.go +++ b/cmd/create_test.go @@ -162,15 +162,15 @@ func TestCreateEmptyWithStagedChanges(t *testing.T) { os.WriteFile(f, []byte("content"), 0644) exec.Command("git", "-C", dir, "add", f).Run() - // Create branch WITHOUT committing (--empty behavior) + // Create branch WITHOUT committing (--no-commit behavior) g.CreateAndCheckout("feature-empty") cfg.SetParent("feature-empty", trunk) - // Don't commit - this simulates --empty flag + // Don't commit - this simulates --no-commit flag // Staged changes should still exist hasStaged, _ := g.HasStagedChanges() if !hasStaged { - t.Error("expected staged changes to remain with --empty") + t.Error("expected staged changes to remain with --no-commit") } } diff --git a/cmd/log.go b/cmd/log.go index d53c163..fa79719 100644 --- a/cmd/log.go +++ b/cmd/log.go @@ -29,8 +29,8 @@ var ( ) func init() { - logCmd.Flags().BoolVar(&logAllFlag, "all", false, "show all branches") - logCmd.Flags().BoolVar(&logPorcelainFlag, "porcelain", false, "machine-readable output") + logCmd.Flags().BoolVarP(&logAllFlag, "all", "a", false, "show all branches") + logCmd.Flags().BoolVarP(&logPorcelainFlag, "porcelain", "p", false, "machine-readable output") rootCmd.AddCommand(logCmd) } diff --git a/cmd/orphan.go b/cmd/orphan.go index 66d5aa9..7b5f9eb 100644 --- a/cmd/orphan.go +++ b/cmd/orphan.go @@ -23,7 +23,7 @@ var orphanCmd = &cobra.Command{ var orphanForceFlag bool func init() { - orphanCmd.Flags().BoolVar(&orphanForceFlag, "force", false, "also orphan all descendants") + orphanCmd.Flags().BoolVarP(&orphanForceFlag, "force", "f", false, "also orphan all descendants") rootCmd.AddCommand(orphanCmd) } @@ -76,6 +76,7 @@ func runOrphan(cmd *cobra.Command, args []string) error { _ = cfg.RemoveParent(desc.Name) //nolint:errcheck // best effort cleanup _ = cfg.RemovePR(desc.Name) //nolint:errcheck // best effort cleanup _ = cfg.RemoveForkPoint(desc.Name) //nolint:errcheck // best effort cleanup + _ = cfg.RemovePRBase(desc.Name) //nolint:errcheck // best effort cleanup fmt.Printf("%s Orphaned %s\n", s.SuccessIcon(), s.Branch(desc.Name)) } } @@ -84,6 +85,7 @@ func runOrphan(cmd *cobra.Command, args []string) error { _ = cfg.RemoveParent(branchName) //nolint:errcheck // best effort cleanup _ = cfg.RemovePR(branchName) //nolint:errcheck // best effort cleanup _ = cfg.RemoveForkPoint(branchName) //nolint:errcheck // best effort cleanup + _ = cfg.RemovePRBase(branchName) //nolint:errcheck // best effort cleanup fmt.Printf("%s Orphaned %s\n", s.SuccessIcon(), s.Branch(branchName)) return nil diff --git a/cmd/cascade.go b/cmd/restack.go similarity index 90% rename from cmd/cascade.go rename to cmd/restack.go index 0a5699e..31fba25 100644 --- a/cmd/cascade.go +++ b/cmd/restack.go @@ -1,4 +1,4 @@ -// cmd/cascade.go +// cmd/restack.go package cmd import ( @@ -15,31 +15,31 @@ import ( "github.com/spf13/cobra" ) -// ErrConflict indicates a rebase conflict occurred during cascade. +// ErrConflict indicates a rebase conflict occurred during restack. var ErrConflict = errors.New("rebase conflict: resolve and run 'gh stack continue', or 'gh stack abort'") -var cascadeCmd = &cobra.Command{ +var restackCmd = &cobra.Command{ Use: "restack", Aliases: []string{"cascade"}, Short: "Rebase current branch and descendants onto their parents", Long: `Rebase the current branch onto its parent, then recursively restack descendants.`, - RunE: runCascade, + RunE: runRestack, } var ( - cascadeOnlyFlag bool - cascadeDryRunFlag bool - cascadeWorktreesFlag bool + restackOnlyFlag bool + restackDryRunFlag bool + restackWorktreesFlag bool ) func init() { - cascadeCmd.Flags().BoolVar(&cascadeOnlyFlag, "only", false, "only restack current branch, not descendants") - cascadeCmd.Flags().BoolVar(&cascadeDryRunFlag, "dry-run", false, "show what would be done") - cascadeCmd.Flags().BoolVar(&cascadeWorktreesFlag, "worktrees", false, "rebase branches checked out in linked worktrees in-place") - rootCmd.AddCommand(cascadeCmd) + restackCmd.Flags().BoolVarP(&restackOnlyFlag, "current", "c", false, "only restack current branch, not descendants") + restackCmd.Flags().BoolVarP(&restackDryRunFlag, "dry-run", "D", false, "show what would be done") + restackCmd.Flags().BoolVarP(&restackWorktreesFlag, "worktrees", "w", false, "rebase branches checked out in linked worktrees in-place") + rootCmd.AddCommand(restackCmd) } -func runCascade(cmd *cobra.Command, args []string) error { +func runRestack(cmd *cobra.Command, args []string) error { s := style.New() cwd, err := os.Getwd() @@ -54,7 +54,7 @@ func runCascade(cmd *cobra.Command, args []string) error { g := git.New(cwd) - // Check if cascade already in progress + // Check if restack already in progress if state.Exists(g.GetGitDir()) { return errors.New("operation already in progress; use 'gh stack continue' or 'gh stack abort'") } @@ -76,18 +76,18 @@ func runCascade(cmd *cobra.Command, args []string) error { 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 + // Collect branches to restack var branches []*tree.Node branches = append(branches, node) - if !cascadeOnlyFlag { + if !restackOnlyFlag { branches = append(branches, tree.GetDescendants(node)...) } // Save undo snapshot (unless dry-run) var stashRef string - if !cascadeDryRunFlag { + if !restackDryRunFlag { var saveErr error - stashRef, saveErr = saveUndoSnapshot(g, cfg, branches, nil, "cascade", "gh stack restack", s) + stashRef, saveErr = saveUndoSnapshot(g, cfg, branches, nil, "restack", "gh stack restack", s) if saveErr != nil { fmt.Printf("%s could not save undo state: %v\n", s.WarningIcon(), saveErr) } @@ -95,7 +95,7 @@ func runCascade(cmd *cobra.Command, args []string) error { // Build worktree map if --worktrees flag is set var worktrees map[string]string - if cascadeWorktreesFlag { + if restackWorktreesFlag { var wtErr error worktrees, wtErr = g.ListWorktrees() if wtErr != nil { @@ -103,9 +103,9 @@ func runCascade(cmd *cobra.Command, args []string) error { } } - err = doCascadeWithState(g, cfg, branches, CascadeOptions{ - DryRun: cascadeDryRunFlag, - Operation: state.OperationCascade, + err = doRestackWithState(g, cfg, branches, RestackOptions{ + DryRun: restackDryRunFlag, + Operation: state.OperationRestack, StashRef: stashRef, Worktrees: worktrees, }, s) @@ -121,15 +121,15 @@ func runCascade(cmd *cobra.Command, args []string) error { return err } -// CascadeOptions configures the behaviour of doCascadeWithState. +// RestackOptions configures the behaviour of doRestackWithState. // // The submit-specific fields (UpdateOnly, OpenWeb, PushOnly, Branches) are // only meaningful when Operation is state.OperationSubmit; they are persisted -// to cascade state so that the push/PR phases can be resumed after a conflict. -type CascadeOptions struct { +// to restack state so that the push/PR phases can be resumed after a conflict. +type RestackOptions struct { // DryRun prints what would be done without actually rebasing. DryRun bool - // Operation is the type of operation being performed (state.OperationCascade + // Operation is the type of operation being performed (state.OperationRestack // or state.OperationSubmit). Operation string // UpdateOnly skips creating new PRs; only existing PRs are updated. @@ -140,8 +140,8 @@ type CascadeOptions struct { // PushOnly skips the PR creation/update phase entirely. Submit-only. PushOnly bool // Branches is the complete list of branch names being submitted, used - // to rebuild the full set for push/PR phases after cascade completes. - // Submit-only. Mirrors state.CascadeState.Branches. + // to rebuild the full set for push/PR phases after restack completes. + // Submit-only. Mirrors state.RestackState.Branches. Branches []string // StashRef is the commit hash of auto-stashed changes (if any), persisted // to state so they can be restored when the operation completes or is aborted. @@ -152,8 +152,8 @@ type CascadeOptions struct { Worktrees map[string]string } -// doCascadeWithState performs cascade and saves state with the given operation type. -func doCascadeWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, opts CascadeOptions, s *style.Style) error { +// doRestackWithState performs restack and saves state with the given operation type. +func doRestackWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, opts RestackOptions, s *style.Style) error { originalBranch, err := g.CurrentBranch() if err != nil { return err @@ -287,7 +287,7 @@ func doCascadeWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o remaining = append(remaining, r.Name) } - st := &state.CascadeState{ + st := &state.RestackState{ Current: b.Name, Pending: remaining, OriginalHead: originalHead, @@ -323,7 +323,7 @@ func doCascadeWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o // Return to original branch if !opts.DryRun { - _ = g.Checkout(originalBranch) //nolint:errcheck // best effort - cascade succeeded + _ = g.Checkout(originalBranch) //nolint:errcheck // best effort - restack succeeded } return nil @@ -332,7 +332,7 @@ func doCascadeWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o // displayOperationName maps internal operation constants to user-facing names. func displayOperationName(op string) string { switch op { - case state.OperationCascade: + case state.OperationRestack: return "Restack" case state.OperationSubmit: return "Submit" diff --git a/cmd/submit.go b/cmd/submit.go index 939f641..4063c80 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -30,7 +30,7 @@ parent-before-child order. Use --from to limit the scope to a subtree. Phases: 1. Restack: rebase affected branches onto their parents -2. Push: force-push all affected branches (with --force-with-lease) +2. Push: force-push branches that will get a PR (or that are bases for one), with --force-with-lease 3. PR: create PRs for branches without them, update PR bases for those that have them If a rebase conflict occurs, resolve it and run 'gh stack continue'.`, @@ -47,17 +47,44 @@ var ( submitFromFlag string ) -// ErrPRSkipped is returned when a user skips PR creation by pressing ESC. -var ErrPRSkipped = errors.New("PR creation skipped by user") +// prAction describes what we will do for a branch in the PR phase after push. +type prAction int + +const ( + prActionSkip prAction = iota + prActionUpdate + prActionAdopt + prActionCreate +) + +// prDecision is the outcome of planning one stack branch before any push. +type prDecision struct { + node *tree.Node + parent string + action prAction + // prActionUpdate + prNum int + // prActionAdopt + adoptPR *github.PR + // prActionCreate + title string + body string + draft bool + // pushAnyway is true when action is prActionSkip but a descendant branch + // will get a PR, so the branch must still exist on the remote as a base. + pushAnyway bool + // skipReason is set when action is prActionSkip ("update" or "user"). + skipReason string +} 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().BoolVar(&submitPushOnlyFlag, "push-only", false, "skip PR creation/update, only restack and push") + submitCmd.Flags().BoolVarP(&submitDryRunFlag, "dry-run", "D", false, "show what would be done without doing it") + submitCmd.Flags().BoolVarP(&submitCurrentOnlyFlag, "current", "c", false, "only submit current branch, not descendants") + submitCmd.Flags().BoolVarP(&submitUpdateOnlyFlag, "update", "u", false, "only update existing PRs, don't create new ones") + submitCmd.Flags().BoolVarP(&submitPushOnlyFlag, "skip-prs", "s", false, "skip PR creation/update, only restack and push") submitCmd.Flags().BoolVarP(&submitYesFlag, "yes", "y", false, "skip interactive prompts and use auto-generated title/description for PRs") - submitCmd.Flags().BoolVarP(&submitWebFlag, "web", "w", false, "open created/updated PRs in web browser") - submitCmd.Flags().StringVar(&submitFromFlag, "from", "", "submit from this branch toward leaves (default: entire stack; bare --from = current branch)") + submitCmd.Flags().BoolVar(&submitWebFlag, "web", false, "open created/updated PRs in web browser") + submitCmd.Flags().StringVarP(&submitFromFlag, "from", "f", "", "submit from this branch toward leaves (default: entire stack; bare --from = current branch)") submitCmd.Flags().Lookup("from").NoOptDefVal = "HEAD" rootCmd.AddCommand(submitCmd) } @@ -67,13 +94,13 @@ func runSubmit(cmd *cobra.Command, args []string) error { // Validate flag combinations if submitPushOnlyFlag && submitUpdateOnlyFlag { - return errors.New("--push-only and --update-only cannot be used together: --push-only skips all PR operations") + return errors.New("--skip-prs and --update cannot be used together: --skip-prs skips all PR operations") } if submitPushOnlyFlag && submitWebFlag { - return errors.New("--push-only and --web cannot be used together: --push-only skips all PR operations") + return errors.New("--skip-prs and --web cannot be used together: --skip-prs skips all PR operations") } if submitFromFlag != "" && submitCurrentOnlyFlag { - return errors.New("--from and --current-only cannot be used together") + return errors.New("--from and --current cannot be used together: --current limits the scope to the current branch") } cwd, err := os.Getwd() @@ -111,15 +138,15 @@ func runSubmit(cmd *cobra.Command, args []string) error { // Collect branches to submit. // - // --current-only: only the current branch (no descendants, no ancestors). + // --current: only the current branch (no descendants, no ancestors). // --from (bare): current branch + descendants (old default behavior). // --from=: that branch + descendants. // Default: entire stack (all trunk descendants). var branches []*tree.Node if submitCurrentOnlyFlag { - // --current-only: submit only the current checked-out branch + // --current: submit only the current checked-out branch if currentBranch == trunk { - return fmt.Errorf("cannot submit trunk branch %q; switch to a stack branch or remove --current-only", trunk) + return fmt.Errorf("cannot submit trunk branch %q; switch to a stack branch or remove --current", trunk) } node := tree.FindNode(root, currentBranch) if node == nil { @@ -177,7 +204,7 @@ func runSubmit(cmd *cobra.Command, args []string) error { // Phase 1: Restack fmt.Println(s.Bold("=== Phase 1: Restack ===")) - if cascadeErr := doCascadeWithState(g, cfg, branches, CascadeOptions{ + if restackErr := doRestackWithState(g, cfg, branches, RestackOptions{ DryRun: submitDryRunFlag, Operation: state.OperationSubmit, UpdateOnly: submitUpdateOnlyFlag, @@ -185,15 +212,15 @@ func runSubmit(cmd *cobra.Command, args []string) error { PushOnly: submitPushOnlyFlag, Branches: branchNames, StashRef: stashRef, - }, s); cascadeErr != nil { + }, s); restackErr != nil { // Stash is saved in state for conflicts; restore on other errors - if !errors.Is(cascadeErr, ErrConflict) && stashRef != "" { + if !errors.Is(restackErr, ErrConflict) && stashRef != "" { fmt.Println("Restoring auto-stashed changes...") if popErr := g.StashPop(stashRef); popErr != nil { fmt.Printf("%s could not restore stashed changes (commit %s): %v\n", s.WarningIcon(), git.AbbrevSHA(stashRef), popErr) } } - return cascadeErr + return restackErr } // Phases 2 & 3 @@ -228,11 +255,43 @@ type SubmitOptions struct { } // doSubmitPushAndPR handles push and PR creation/update phases. -// This is called after cascade succeeds (or from continue after conflict resolution). +// This is called after restack succeeds (or from continue after conflict resolution). func doSubmitPushAndPR(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tree.Node, opts SubmitOptions, s *style.Style) error { - // Phase 2: Push all branches + var decisions []*prDecision + var ghClient *github.Client + if !opts.PushOnly && !opts.DryRun { + var clientErr error + ghClient, clientErr = github.NewClient() + if clientErr != nil { + return clientErr + } + } + if !opts.PushOnly { + trunk, err := cfg.GetTrunk() + if err != nil { + return err + } + decisions = planPRDecisions(g, cfg, ghClient, trunk, branches, opts.DryRun, opts.UpdateOnly, s) + applyMustPushForSkippedAncestors(decisions) + } + + decisionByName := make(map[string]*prDecision, len(decisions)) + for _, d := range decisions { + decisionByName[d.node.Name] = d + } + + // Phase 2: Push branches that will participate in PRs (or all if --skip-prs). fmt.Println(s.Bold("\n=== Phase 2: Push ===")) for _, b := range branches { + var d *prDecision + if !opts.PushOnly { + d = decisionByName[b.Name] + } + shouldPush := opts.PushOnly || d == nil || d.action != prActionSkip || d.pushAnyway + if !shouldPush { + fmt.Printf("Skipping push for %s %s\n", s.Branch(b.Name), s.Muted("(no PR for this branch)")) + continue + } if opts.DryRun { fmt.Printf("%s Would push %s -> origin/%s (forced)\n", s.Muted("dry-run:"), s.Branch(b.Name), s.Branch(b.Name)) } else { @@ -245,17 +304,106 @@ func doSubmitPushAndPR(g *git.Git, cfg *config.Config, root *tree.Node, branches } } - // Phase 3: Create/update PRs if opts.PushOnly { fmt.Println(s.Bold("\n=== Phase 3: PRs ===")) - fmt.Println(s.Muted("Skipped (--push-only)")) + fmt.Println(s.Muted("Skipped (--skip-prs)")) return nil } - return doSubmitPRs(g, cfg, root, branches, opts, s) + return executePRDecisions(g, cfg, root, decisions, ghClient, opts, s) +} + +// planPRDecisions resolves what to do for each branch before any push. +// When dryRun is true, ghClient may be nil (no GitHub lookups for adopt). +func planPRDecisions(g *git.Git, cfg *config.Config, ghClient *github.Client, trunk string, branches []*tree.Node, dryRun, updateOnly bool, s *style.Style) []*prDecision { + out := make([]*prDecision, 0, len(branches)) + for _, b := range branches { + parent, _ := cfg.GetParent(b.Name) //nolint:errcheck // empty is fine + if parent == "" { + parent = trunk + } + existingPR, _ := cfg.GetPR(b.Name) //nolint:errcheck // 0 is fine + switch { + case existingPR > 0: + out = append(out, &prDecision{node: b, parent: parent, action: prActionUpdate, prNum: existingPR}) + case updateOnly: + out = append(out, &prDecision{node: b, parent: parent, action: prActionSkip, skipReason: "update"}) + case dryRun: + out = append(out, planPRDecisionDryRun(g, b, parent, trunk, s)) + default: + out = append(out, planPRDecisionInteractive(g, ghClient, b, parent, trunk, s)) + } + } + return out +} + +func planPRDecisionDryRun(g *git.Git, b *tree.Node, parent, trunk string, s *style.Style) *prDecision { + draft := parent != trunk + defaultTitle := generateDefaultTitle(g, parent, b.Name) + defaultBody, bodyErr := generatePRBody(g, parent, b.Name) + if bodyErr != nil { + fmt.Printf("%s could not generate PR body: %v\n", s.WarningIcon(), bodyErr) + defaultBody = "" + } + return &prDecision{ + node: b, + parent: parent, + action: prActionCreate, + title: defaultTitle, + body: defaultBody, + draft: draft, + } +} + +func planPRDecisionInteractive(g *git.Git, ghClient *github.Client, b *tree.Node, parent, trunk string, s *style.Style) *prDecision { + branch := b.Name + existingPR, err := ghClient.FindPRByHead(branch) + if err != nil { + fmt.Printf("%s could not check for existing PR: %v\n", s.WarningIcon(), err) + } else if existingPR != nil { + return &prDecision{node: b, parent: parent, action: prActionAdopt, adoptPR: existingPR} + } + + draft := parent != trunk + defaultTitle := generateDefaultTitle(g, parent, branch) + defaultBody, bodyErr := generatePRBody(g, parent, branch) + if bodyErr != nil { + fmt.Printf("%s could not generate PR body: %v\n", s.WarningIcon(), bodyErr) + defaultBody = "" + } + title, body, skipped, err := promptForPRDetails(branch, defaultTitle, defaultBody, s) + if err != nil { + fmt.Printf("%s failed to get PR details for %s: %v\n", s.WarningIcon(), s.Branch(branch), err) + return &prDecision{node: b, parent: parent, action: prActionSkip, skipReason: "user"} + } + if skipped { + return &prDecision{node: b, parent: parent, action: prActionSkip, skipReason: "user"} + } + return &prDecision{node: b, parent: parent, action: prActionCreate, title: title, body: body, draft: draft} +} + +// applyMustPushForSkippedAncestors marks skipped branches that must still be +// pushed because a descendant will open or update a PR that uses them as base. +func applyMustPushForSkippedAncestors(decisions []*prDecision) { + byName := make(map[string]*prDecision, len(decisions)) + for _, d := range decisions { + byName[d.node.Name] = d + } + for _, d := range decisions { + if d.action == prActionSkip { + continue + } + for cur := d.node.Parent; cur != nil; cur = cur.Parent { + if x := byName[cur.Name]; x != nil && x.action == prActionSkip { + x.pushAnyway = true + } + } + } } -// doSubmitPRs handles PR creation/update for all branches. -func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tree.Node, opts SubmitOptions, s *style.Style) error { +// executePRDecisions runs the PR phase from pre-planned decisions (after push). +// planningClient is the client used during planPRDecisions; it is nil in dry-run. +// When not dry-run, the same client is reused here (no second NewClient). +func executePRDecisions(g *git.Git, cfg *config.Config, root *tree.Node, decisions []*prDecision, planningClient *github.Client, opts SubmitOptions, s *style.Style) error { fmt.Println(s.Bold("\n=== Phase 3: PRs ===")) trunk, err := cfg.GetTrunk() @@ -263,9 +411,8 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr return err } - // In dry-run mode, we don't need a GitHub client - var ghClient *github.Client - if !opts.DryRun { + ghClient := planningClient + if !opts.DryRun && ghClient == nil { var clientErr error ghClient, clientErr = github.NewClient() if clientErr != nil { @@ -273,19 +420,15 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr } } - // Build remote branches set for stack comment filtering. - // This uses locally-cached tracking refs which are up-to-date after Phase 2 push. var remoteBranches map[string]bool if !opts.DryRun { var rbErr error remoteBranches, rbErr = g.ListRemoteBranches() if rbErr != nil { - // Non-fatal: we can still render without filtering fmt.Printf("%s could not list remote branches, stack comments may reference local-only branches: %v\n", s.WarningIcon(), rbErr) } } - // Build the PR context once - shared across all branches in this submit run. pCtx := prContext{ ghClient: ghClient, cfg: cfg, @@ -295,75 +438,91 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr s: s, } - // Collect PR URLs for --web flag var prURLs []string - for _, b := range branches { - parent, _ := cfg.GetParent(b.Name) //nolint:errcheck // empty is fine - if parent == "" { - parent = trunk - } - - existingPR, _ := cfg.GetPR(b.Name) //nolint:errcheck // 0 is fine + for _, d := range decisions { + b := d.node + parent := d.parent - switch { - case existingPR > 0: - // Update existing PR + switch d.action { + case prActionSkip: if opts.DryRun { - fmt.Printf("%s Would update PR #%d base to %s\n", pCtx.s.Muted("dry-run:"), existingPR, pCtx.s.Branch(parent)) + if d.skipReason == "update" { + fmt.Printf("Skipping %s %s\n", s.Branch(b.Name), s.Muted("(no existing PR, --update)")) + } + continue + } + switch d.skipReason { + case "update": + fmt.Printf("Skipping %s %s\n", s.Branch(b.Name), s.Muted("(no existing PR, --update)")) + case "user": + fmt.Printf("Skipped PR for %s %s\n", s.Branch(b.Name), s.Muted("(skipped)")) + } + case prActionUpdate: + if opts.DryRun { + fmt.Printf("%s Would update PR #%d base to %s\n", s.Muted("dry-run:"), d.prNum, s.Branch(parent)) } else { - fmt.Printf("Updating PR #%d for %s (base: %s)... ", existingPR, pCtx.s.Branch(b.Name), pCtx.s.Branch(parent)) - if err := pCtx.ghClient.UpdatePRBase(existingPR, parent); err != nil { - fmt.Println(pCtx.s.Error("failed")) - fmt.Printf("%s failed to update PR #%d base: %v\n", pCtx.s.WarningIcon(), existingPR, err) + fmt.Printf("Updating PR #%d for %s (base: %s)... ", d.prNum, s.Branch(b.Name), s.Branch(parent)) + if err := ghClient.UpdatePRBase(d.prNum, parent); err != nil { + fmt.Println(s.Error("failed")) + fmt.Printf("%s failed to update PR #%d base: %v\n", s.WarningIcon(), d.prNum, err) } else { - fmt.Println(pCtx.s.Success("ok")) + fmt.Println(s.Success("ok")) + // Check for trunk transition BEFORE persisting the new base, so + // isTransitionToTrunk compares against the previous stored value. + maybeMarkPRReady(ghClient, cfg, d.prNum, b.Name, parent, trunk, s) + _ = cfg.SetPRBase(b.Name, parent) //nolint:errcheck // best effort — used for transition detection only + if err := ghClient.GenerateAndPostStackComment(root, b.Name, trunk, d.prNum, remoteBranches); err != nil { + fmt.Printf("%s failed to update stack comment for PR #%d: %v\n", s.WarningIcon(), d.prNum, err) + } if opts.OpenWeb { - prURLs = append(prURLs, pCtx.ghClient.PRURL(existingPR)) + prURLs = append(prURLs, ghClient.PRURL(d.prNum)) } } - // Update stack comment - if err := pCtx.ghClient.GenerateAndPostStackComment(pCtx.root, b.Name, pCtx.trunk, existingPR, pCtx.remoteBranches); err != nil { - fmt.Printf("%s failed to update stack comment for PR #%d: %v\n", pCtx.s.WarningIcon(), existingPR, err) + } + case prActionAdopt: + if opts.DryRun { + fmt.Printf("%s Would adopt PR #%d for %s (base: %s)\n", s.Muted("dry-run:"), d.adoptPR.Number, s.Branch(b.Name), s.Branch(parent)) + } else { + prNum, adoptErr := adoptExistingPRDirect(pCtx, b.Name, parent, d.adoptPR) + switch { + case adoptErr != nil: + fmt.Printf("%s failed to adopt PR for %s: %v\n", s.WarningIcon(), s.Branch(b.Name), adoptErr) + default: + fmt.Printf("%s Adopted PR #%d for %s (%s)\n", s.SuccessIcon(), prNum, s.Branch(b.Name), ghClient.PRURL(prNum)) + if opts.OpenWeb { + prURLs = append(prURLs, ghClient.PRURL(prNum)) + } } - - // If PR is a draft and now targets trunk, offer to publish - maybeMarkPRReady(pCtx.ghClient, existingPR, b.Name, parent, pCtx.trunk, pCtx.s) } - case !opts.UpdateOnly: - // Create new PR + case prActionCreate: if opts.DryRun { - fmt.Printf("%s Would create PR for %s (base: %s)\n", pCtx.s.Muted("dry-run:"), pCtx.s.Branch(b.Name), pCtx.s.Branch(parent)) + fmt.Printf("%s Would create PR for %s (base: %s)\n", s.Muted("dry-run:"), s.Branch(b.Name), s.Branch(parent)) } else { - prNum, adopted, err := createPRForBranch(g, pCtx, b.Name, parent) + prNum, adopted, execErr := executePRCreate(pCtx, b.Name, parent, d.title, d.body, d.draft) switch { - case errors.Is(err, ErrPRSkipped): - fmt.Printf("Skipped PR for %s %s\n", pCtx.s.Branch(b.Name), pCtx.s.Muted("(skipped)")) - case err != nil: - fmt.Printf("%s failed to create PR for %s: %v\n", pCtx.s.WarningIcon(), pCtx.s.Branch(b.Name), err) + case execErr != nil: + fmt.Printf("%s failed to create PR for %s: %v\n", s.WarningIcon(), s.Branch(b.Name), execErr) case adopted: - fmt.Printf("%s Adopted PR #%d for %s (%s)\n", pCtx.s.SuccessIcon(), prNum, pCtx.s.Branch(b.Name), pCtx.ghClient.PRURL(prNum)) + fmt.Printf("%s Adopted PR #%d for %s (%s)\n", s.SuccessIcon(), prNum, s.Branch(b.Name), ghClient.PRURL(prNum)) if opts.OpenWeb { - prURLs = append(prURLs, pCtx.ghClient.PRURL(prNum)) + prURLs = append(prURLs, ghClient.PRURL(prNum)) } default: - fmt.Printf("%s Created PR #%d for %s (%s)\n", pCtx.s.SuccessIcon(), prNum, pCtx.s.Branch(b.Name), pCtx.ghClient.PRURL(prNum)) + fmt.Printf("%s Created PR #%d for %s (%s)\n", s.SuccessIcon(), prNum, s.Branch(b.Name), ghClient.PRURL(prNum)) if opts.OpenWeb { - prURLs = append(prURLs, pCtx.ghClient.PRURL(prNum)) + prURLs = append(prURLs, ghClient.PRURL(prNum)) } } } - default: - fmt.Printf("Skipping %s %s\n", pCtx.s.Branch(b.Name), pCtx.s.Muted("(no existing PR, --update-only)")) } } - // Open PRs in browser if requested if opts.OpenWeb && len(prURLs) > 0 { - b := browser.New("", os.Stdout, os.Stderr) + brw := browser.New("", os.Stdout, os.Stderr) for _, url := range prURLs { - if err := b.Browse(url); err != nil { - fmt.Fprintf(os.Stderr, "%s could not open browser for %s: %v\n", pCtx.s.WarningIcon(), url, err) + if err := brw.Browse(url); err != nil { + fmt.Fprintf(os.Stderr, "%s could not open browser for %s: %v\n", s.WarningIcon(), url, err) } } } @@ -384,53 +543,15 @@ type prContext struct { s *style.Style } -// createPRForBranch creates a PR for the given branch and stores the PR number. -// 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, pCtx prContext, branch, base string) (int, bool, error) { - // Check for existing PR on GitHub BEFORE prompting user for title/description. - // This avoids the confusing UX where we prompt then immediately say "oh, PR already exists". - existingPR, err := pCtx.ghClient.FindPRByHead(branch) - if err != nil { - // Non-fatal: proceed with creation attempt which will give clearer error if needed - fmt.Printf("%s could not check for existing PR: %v\n", pCtx.s.WarningIcon(), err) - } else if existingPR != nil { - // PR exists on GitHub but wasn't in local config - adopt it without prompting - prNum, adoptErr := adoptExistingPRDirect(pCtx, branch, base, existingPR) - return prNum, true, adoptErr - } - - // Determine if draft (not targeting trunk = middle of stack) - draft := base != pCtx.trunk - - // Generate default title from first commit message (falls back to branch name) - defaultTitle := generateDefaultTitle(g, base, branch) - - // Generate PR body from commits - defaultBody, bodyErr := generatePRBody(g, base, branch) - if bodyErr != nil { - // Non-fatal: just skip auto-body - fmt.Printf("%s could not generate PR body: %v\n", pCtx.s.WarningIcon(), bodyErr) - defaultBody = "" - } - - // Get title and body (prompt if interactive and --yes not set) - title, body, skipped, err := promptForPRDetails(branch, defaultTitle, defaultBody, pCtx.s) - if err != nil { - return 0, false, fmt.Errorf("failed to get PR details: %w", err) - } - if skipped { - return 0, false, ErrPRSkipped - } - +// executePRCreate opens a PR with the given title/body (branch must be on the remote). +// Returns adopted true if an existing PR was adopted instead of creating. +func executePRCreate(pCtx prContext, branch, base, title, body string, draft bool) (prNum int, adopted bool, err error) { pr, err := pCtx.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(pCtx, branch, base) return prNum, true, adoptErr } - // Detect missing base branch on remote and provide an actionable message if isBaseBranchInvalidError(err) { return 0, false, fmt.Errorf( "base branch %q does not exist on the remote; push it first or run 'gh stack submit' to push the entire stack: %w", @@ -439,17 +560,17 @@ func createPRForBranch(g *git.Git, pCtx prContext, branch, base string) (int, bo return 0, false, err } - // Store PR number in config if err := pCtx.cfg.SetPR(branch, pr.Number); err != nil { 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 + // Persist the base so future submit runs can detect trunk transitions. + _ = pCtx.cfg.SetPRBase(branch, base) //nolint:errcheck // best effort + if node := tree.FindNode(pCtx.root, branch); node != nil { node.PR = pr.Number } - // Add stack navigation comment if err := pCtx.ghClient.GenerateAndPostStackComment(pCtx.root, branch, pCtx.trunk, pr.Number, pCtx.remoteBranches); err != nil { fmt.Printf("%s failed to add stack comment to PR #%d: %v\n", pCtx.s.WarningIcon(), pr.Number, err) } @@ -577,7 +698,7 @@ func adoptExistingPR(pCtx prContext, branch, base string) (int, error) { } // adoptExistingPRDirect adopts an already-fetched PR into the stack. -// This is the implementation shared by adoptExistingPR and the early-detection path in createPRForBranch. +// This is the implementation shared by adoptExistingPR and the adopt path in executePRDecisions. func adoptExistingPRDirect(pCtx prContext, branch, base string, existingPR *github.PR) (int, error) { // Store PR number in config if err := pCtx.cfg.SetPR(branch, existingPR.Number); err != nil { @@ -596,23 +717,51 @@ func adoptExistingPRDirect(pCtx prContext, branch, base string, existingPR *gith } } + // Persist the new base for transition detection on future submit runs. + _ = pCtx.cfg.SetPRBase(branch, base) //nolint:errcheck // best effort + // Add/update stack navigation comment if err := pCtx.ghClient.GenerateAndPostStackComment(pCtx.root, branch, pCtx.trunk, existingPR.Number, pCtx.remoteBranches); err != nil { fmt.Printf("%s failed to update stack comment: %v\n", pCtx.s.WarningIcon(), err) } - // If adopted PR is a draft and targets trunk, offer to publish - if existingPR.Draft && base == pCtx.trunk { + // Prompt to publish if the PR is a draft and its base is transitioning to trunk. + // We use the live pre-adoption base (existingPR.Base.Ref) here since stored base + // metadata may not exist yet for a freshly adopted PR. + if existingPR.Draft && base == pCtx.trunk && existingPR.Base.Ref != pCtx.trunk { promptMarkPRReady(pCtx.ghClient, existingPR.Number, branch, pCtx.trunk, pCtx.s) } return existingPR.Number, nil } +// isTransitionToTrunk reports whether a PR's base branch is moving to trunk for +// the first time as tracked by gh-stack. It uses the stored last-known PR base +// (persisted after each successful submit) to detect the transition, avoiding +// any additional GitHub API calls. +// +// Returns true when: +// - no stored base exists yet (first run after PR creation/adoption), or +// - stored base is not trunk (base is changing from something else to trunk). +// +// Returns false when the stored base is already trunk, meaning the PR was +// already targeting trunk on the previous submit run. +func isTransitionToTrunk(cfg *config.Config, branch, trunk string) bool { + storedBase, err := cfg.GetPRBase(branch) + if err == nil && storedBase == trunk { + return false + } + return true +} + // maybeMarkPRReady checks if a PR is a draft targeting trunk and offers to publish it. // This handles the case where a PR was created as a draft (middle of stack) but now // targets trunk because its parent was merged. -func maybeMarkPRReady(ghClient *github.Client, prNumber int, branch, base, trunk string, s *style.Style) { +// +// The prompt fires only when the base branch is transitioning to trunk for the first +// time — i.e., the stored last-known base is not already trunk. This prevents the +// prompt from appearing on every submit run once the PR already targets trunk. +func maybeMarkPRReady(ghClient *github.Client, cfg *config.Config, prNumber int, branch, base, trunk string, s *style.Style) { // Only relevant if PR now targets trunk if base != trunk { return @@ -624,6 +773,11 @@ func maybeMarkPRReady(ghClient *github.Client, prNumber int, branch, base, trunk return } + // Only prompt when the base is transitioning to trunk this run. + if !isTransitionToTrunk(cfg, branch, trunk) { + return + } + promptMarkPRReady(ghClient, prNumber, branch, trunk, s) } @@ -633,9 +787,9 @@ func promptMarkPRReady(ghClient *github.Client, prNumber int, branch, trunk stri fmt.Printf("PR #%d (%s) is a draft and now targets %s.\n", prNumber, s.Branch(branch), s.Branch(trunk)) // Skip prompt if --yes flag is set or non-interactive - shouldMarkReady := true + shouldMarkReady := false if !submitYesFlag && prompt.IsInteractive() { - shouldMarkReady, _ = prompt.Confirm("Mark as ready for review?", true) //nolint:errcheck // default is fine + shouldMarkReady, _ = prompt.Confirm("Mark as ready for review?", false) //nolint:errcheck // default is fine } if shouldMarkReady { diff --git a/cmd/submit_internal_test.go b/cmd/submit_internal_test.go index c3b2b11..463c81c 100644 --- a/cmd/submit_internal_test.go +++ b/cmd/submit_internal_test.go @@ -9,7 +9,14 @@ package cmd import ( "errors" "fmt" + "os/exec" "testing" + + "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/style" + "github.com/boneskull/gh-stack/internal/tree" ) func TestUnwrapParagraphs(t *testing.T) { @@ -315,3 +322,294 @@ func TestIsBaseBranchInvalidError(t *testing.T) { }) } } + +func setupTestRepo(t *testing.T) *config.Config { + t.Helper() + dir := t.TempDir() + if err := exec.Command("git", "init", dir).Run(); err != nil { + t.Fatalf("git init failed: %v", err) + } + exec.Command("git", "-C", dir, "config", "user.email", "test@test.com").Run() //nolint:errcheck + exec.Command("git", "-C", dir, "config", "user.name", "Test").Run() //nolint:errcheck + cfg, err := config.Load(dir) + if err != nil { + t.Fatalf("config.Load failed: %v", err) + } + return cfg +} + +// setupTestRepoWithDir is like setupTestRepo but also returns the directory path +// for callers that need to run git commands directly or construct a git.Git instance. +func setupTestRepoWithDir(t *testing.T) (*config.Config, string) { + t.Helper() + dir := t.TempDir() + if err := exec.Command("git", "init", dir).Run(); err != nil { + t.Fatalf("git init failed: %v", err) + } + exec.Command("git", "-C", dir, "config", "user.email", "test@test.com").Run() //nolint:errcheck + exec.Command("git", "-C", dir, "config", "user.name", "Test").Run() //nolint:errcheck + // Create an initial commit so the repo has a HEAD and we can create branches. + exec.Command("git", "-C", dir, "commit", "--allow-empty", "-m", "init").Run() //nolint:errcheck + cfg, err := config.Load(dir) + if err != nil { + t.Fatalf("config.Load failed: %v", err) + } + return cfg, dir +} + +func TestIsTransitionToTrunk(t *testing.T) { + trunk := "main" + + t.Run("no_stored_base_returns_true", func(t *testing.T) { + cfg := setupTestRepo(t) + // No stored base — first run after PR creation; should prompt. + if !isTransitionToTrunk(cfg, "feat-a", trunk) { + t.Error("expected true when no stored base exists") + } + }) + + t.Run("stored_base_is_not_trunk_returns_true", func(t *testing.T) { + cfg := setupTestRepo(t) + // Branch previously targeted a non-trunk parent; now it targets trunk. + if err := cfg.SetPRBase("feat-a", "feat-parent"); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + if !isTransitionToTrunk(cfg, "feat-a", trunk) { + t.Error("expected true when stored base is not trunk") + } + }) + + t.Run("stored_base_is_trunk_returns_false", func(t *testing.T) { + cfg := setupTestRepo(t) + // Branch was already targeting trunk on the previous run; don't re-prompt. + if err := cfg.SetPRBase("feat-a", trunk); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + if isTransitionToTrunk(cfg, "feat-a", trunk) { + t.Error("expected false when stored base is already trunk") + } + }) + + t.Run("different_branches_are_independent", func(t *testing.T) { + cfg := setupTestRepo(t) + // feat-a already targeting trunk; feat-b has no stored base. + if err := cfg.SetPRBase("feat-a", trunk); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + if isTransitionToTrunk(cfg, "feat-a", trunk) { + t.Error("feat-a: expected false when stored base is trunk") + } + if !isTransitionToTrunk(cfg, "feat-b", trunk) { + t.Error("feat-b: expected true when no stored base exists") + } + }) + + t.Run("custom_trunk_name_works", func(t *testing.T) { + cfg := setupTestRepo(t) + customTrunk := "master" + // Stored as "master"; should not prompt. + if err := cfg.SetPRBase("feat-a", customTrunk); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + if isTransitionToTrunk(cfg, "feat-a", customTrunk) { + t.Error("expected false with custom trunk name already stored") + } + // Stored as something else; should prompt. + if err := cfg.SetPRBase("feat-b", "other-branch"); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + if !isTransitionToTrunk(cfg, "feat-b", customTrunk) { + t.Error("expected true when stored base is not the custom trunk") + } + }) +} + +// TestIsTransitionToTrunkOrderingInvariant verifies the property that +// isTransitionToTrunk must be evaluated BEFORE SetPRBase is called with trunk, +// because SetPRBase overwrites the stored value that the function reads. +// This guards against the regression where SetPRBase was called before +// maybeMarkPRReady in the prActionUpdate path, silently suppressing the prompt. +func TestIsTransitionToTrunkOrderingInvariant(t *testing.T) { + trunk := "main" + + t.Run("returns_true_before_SetPRBase_when_parent_stored", func(t *testing.T) { + cfg := setupTestRepo(t) + if err := cfg.SetPRBase("feat-a", "feat-parent"); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + // Simulates the correct ordering: check transition BEFORE persisting trunk. + transitionDetected := isTransitionToTrunk(cfg, "feat-a", trunk) + _ = cfg.SetPRBase("feat-a", trunk) + if !transitionDetected { + t.Error("expected transition to be detected when checked before SetPRBase(trunk)") + } + }) + + t.Run("returns_false_after_SetPRBase_wrong_order", func(t *testing.T) { + cfg := setupTestRepo(t) + if err := cfg.SetPRBase("feat-a", "feat-parent"); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + // Simulates the buggy ordering: persist trunk BEFORE checking transition. + _ = cfg.SetPRBase("feat-a", trunk) + transitionDetected := isTransitionToTrunk(cfg, "feat-a", trunk) + if transitionDetected { + t.Error("demonstrates the bug: SetPRBase before check suppresses the prompt") + } + }) +} + +func TestApplyMustPushForSkippedAncestors(t *testing.T) { + main := &tree.Node{Name: "main"} + featA := &tree.Node{Name: "feat-a", Parent: main} + featB := &tree.Node{Name: "feat-b", Parent: featA} + + t.Run("skipped_parent_pushed_when_child_gets_PR", func(t *testing.T) { + decisions := []*prDecision{ + {node: featA, action: prActionSkip, skipReason: "user"}, + {node: featB, action: prActionCreate, title: "t", body: "b", draft: false}, + } + applyMustPushForSkippedAncestors(decisions) + if !decisions[0].pushAnyway { + t.Error("feat-a should be marked pushAnyway when feat-b gets a PR") + } + }) + + t.Run("skipped_branch_not_marked_when_no_descendant_PR", func(t *testing.T) { + decisions := []*prDecision{ + {node: featA, action: prActionSkip, skipReason: "user"}, + {node: featB, action: prActionSkip, skipReason: "user"}, + } + applyMustPushForSkippedAncestors(decisions) + if decisions[0].pushAnyway || decisions[1].pushAnyway { + t.Error("no pushAnyway when entire subtree is skipped") + } + }) + + t.Run("skipped_parent_when_child_updates_PR", func(t *testing.T) { + decisions := []*prDecision{ + {node: featA, action: prActionSkip, skipReason: "user"}, + {node: featB, action: prActionUpdate, prNum: 42}, + } + applyMustPushForSkippedAncestors(decisions) + if !decisions[0].pushAnyway { + t.Error("feat-a should be pushAnyway when feat-b updates a PR") + } + }) + + t.Run("skipped_parent_when_child_adopts_PR", func(t *testing.T) { + decisions := []*prDecision{ + {node: featA, action: prActionSkip, skipReason: "user"}, + {node: featB, action: prActionAdopt, adoptPR: &github.PR{Number: 7}}, + } + applyMustPushForSkippedAncestors(decisions) + if !decisions[0].pushAnyway { + t.Error("feat-a should be pushAnyway when feat-b adopts a PR") + } + }) + + t.Run("three_level_chain_both_skipped_ancestors", func(t *testing.T) { + featC := &tree.Node{Name: "feat-c", Parent: featB} + decisions := []*prDecision{ + {node: featA, action: prActionSkip, skipReason: "user"}, + {node: featB, action: prActionSkip, skipReason: "user"}, + {node: featC, action: prActionCreate, title: "t", body: "b", draft: false}, + } + applyMustPushForSkippedAncestors(decisions) + if !decisions[0].pushAnyway || !decisions[1].pushAnyway { + t.Errorf("both ancestors should be pushAnyway, got feat-a=%v feat-b=%v", decisions[0].pushAnyway, decisions[1].pushAnyway) + } + }) + + t.Run("non_skipped_ancestor_not_given_pushAnyway_flag", func(t *testing.T) { + decisions := []*prDecision{ + {node: featA, action: prActionUpdate, prNum: 1}, + {node: featB, action: prActionCreate, title: "t", body: "b", draft: false}, + } + applyMustPushForSkippedAncestors(decisions) + if decisions[0].pushAnyway { + t.Error("feat-a is not skipped; pushAnyway should stay false") + } + }) +} + +func TestDeleteMergedBranchClearsPRBase(t *testing.T) { + cfg, dir := setupTestRepoWithDir(t) + g := git.New(dir) + s := style.New() + + 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) + } + + // Create feature-a with a commit so git can delete it later. + if err := exec.Command("git", "-C", dir, "checkout", "-b", "feature-a").Run(); err != nil { + t.Fatalf("create branch failed: %v", err) + } + if err := exec.Command("git", "-C", dir, "commit", "--allow-empty", "-m", "feat").Run(); err != nil { + t.Fatalf("commit failed: %v", err) + } + if err := exec.Command("git", "-C", dir, "checkout", trunk).Run(); err != nil { + t.Fatalf("checkout trunk failed: %v", err) + } + + if err := cfg.SetParent("feature-a", trunk); err != nil { + t.Fatalf("SetParent failed: %v", err) + } + if err := cfg.SetPR("feature-a", 10); err != nil { + t.Fatalf("SetPR failed: %v", err) + } + if err := cfg.SetPRBase("feature-a", trunk); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + + currentBranch := trunk + deleteMergedBranch(g, cfg, "feature-a", trunk, ¤tBranch, s) + + if v, err := cfg.GetPRBase("feature-a"); err == nil { + t.Errorf("expected stackPRBase to be removed after deleteMergedBranch, got %q", v) + } + if v, err := cfg.GetPR("feature-a"); err == nil { + t.Errorf("expected stackPR to be removed after deleteMergedBranch, got %d", v) + } +} + +func TestOrphanMergedBranchClearsPRBase(t *testing.T) { + cfg, dir := setupTestRepoWithDir(t) + g := git.New(dir) + s := style.New() + + 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) + } + + if err := cfg.SetParent("feature-a", trunk); err != nil { + t.Fatalf("SetParent failed: %v", err) + } + if err := cfg.SetPR("feature-a", 11); err != nil { + t.Fatalf("SetPR failed: %v", err) + } + if err := cfg.SetPRBase("feature-a", trunk); err != nil { + t.Fatalf("SetPRBase failed: %v", err) + } + + orphanMergedBranch(cfg, "feature-a", s) + + if v, err := cfg.GetPRBase("feature-a"); err == nil { + t.Errorf("expected stackPRBase to be removed after orphanMergedBranch, got %q", v) + } + if v, err := cfg.GetPR("feature-a"); err == nil { + t.Errorf("expected stackPR to be removed after orphanMergedBranch, got %d", v) + } + if v, err := cfg.GetParent("feature-a"); err == nil { + t.Errorf("expected stackParent to be removed after orphanMergedBranch, got %q", v) + } +} diff --git a/cmd/sync.go b/cmd/sync.go index 049774b..aa40ee5 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -25,15 +25,15 @@ var syncCmd = &cobra.Command{ } var ( - syncNoCascadeFlag bool + syncNoRestackFlag bool syncDryRunFlag bool syncWorktreesFlag bool ) func init() { - syncCmd.Flags().BoolVar(&syncNoCascadeFlag, "no-restack", false, "skip restacking branches") - syncCmd.Flags().BoolVar(&syncDryRunFlag, "dry-run", false, "show what would be done") - syncCmd.Flags().BoolVar(&syncWorktreesFlag, "worktrees", false, "rebase branches checked out in linked worktrees in-place") + syncCmd.Flags().BoolVar(&syncNoRestackFlag, "no-restack", false, "skip restacking branches") + syncCmd.Flags().BoolVarP(&syncDryRunFlag, "dry-run", "D", false, "show what would be done") + syncCmd.Flags().BoolVarP(&syncWorktreesFlag, "worktrees", "w", false, "rebase branches checked out in linked worktrees in-place") rootCmd.AddCommand(syncCmd) } @@ -202,7 +202,7 @@ func runSync(cmd *cobra.Command, args []string) error { // 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 + // Can't determine, let restack try continue } @@ -318,6 +318,10 @@ func runSync(cmd *cobra.Command, args []string) error { if rt.childPR > 0 { if updateErr := gh.UpdatePRBase(rt.childPR, trunk); updateErr != nil { fmt.Printf("%s failed to update PR #%d base: %v\n", s.WarningIcon(), rt.childPR, updateErr) + } else { + // Persist the new base so the submit publish-prompt knows this + // PR is already targeting trunk on its next run. + _ = cfg.SetPRBase(rt.childName, trunk) //nolint:errcheck // best effort } } @@ -330,7 +334,7 @@ func runSync(cmd *cobra.Command, args []string) error { fmt.Printf("Rebasing %s onto %s (from fork point %s)...\n", s.Branch(rt.childName), s.Branch(trunk), displayForkPoint) if rebaseErr := g.RebaseOnto(trunk, rt.forkPoint, rt.childName); rebaseErr != nil { fmt.Printf("%s --onto rebase failed, will try normal restack: %v\n", s.WarningIcon(), rebaseErr) - // Don't return error - let cascade try + // Don't return error - let restack try } else { fmt.Printf("%s Rebased %s successfully\n", s.SuccessIcon(), s.Branch(rt.childName)) @@ -348,8 +352,8 @@ func runSync(cmd *cobra.Command, args []string) error { _ = g.Checkout(currentBranch) //nolint:errcheck // best effort } - // Cascade all (if not disabled) - if !syncNoCascadeFlag { + // Restack all (if not disabled) + if !syncNoRestackFlag { fmt.Println(s.Bold("\nRestacking all branches...")) // Rebuild tree after modifications root, err = tree.Build(cfg) @@ -367,13 +371,13 @@ func runSync(cmd *cobra.Command, args []string) error { } } - // Cascade from trunk's children + // Restack from trunk's children for _, child := range root.Children { allBranches := []*tree.Node{child} allBranches = append(allBranches, tree.GetDescendants(child)...) - if err := doCascadeWithState(g, cfg, allBranches, CascadeOptions{ + if err := doRestackWithState(g, cfg, allBranches, RestackOptions{ DryRun: syncDryRunFlag, - Operation: state.OperationCascade, + Operation: state.OperationRestack, StashRef: stashRef, Worktrees: worktrees, }, s); err != nil { @@ -473,6 +477,7 @@ func deleteMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, cu _ = cfg.RemoveParent(branch) //nolint:errcheck // best effort cleanup _ = cfg.RemovePR(branch) //nolint:errcheck // best effort cleanup _ = cfg.RemoveForkPoint(branch) //nolint:errcheck // best effort cleanup + _ = cfg.RemovePRBase(branch) //nolint:errcheck // best effort cleanup if err := g.DeleteBranch(branch); err != nil { fmt.Printf("%s could not delete branch %s: %v\n", s.WarningIcon(), s.Branch(branch), err) } @@ -485,5 +490,6 @@ func orphanMergedBranch(cfg *config.Config, branch string, s *style.Style) merge _ = cfg.RemoveParent(branch) //nolint:errcheck // best effort cleanup _ = cfg.RemovePR(branch) //nolint:errcheck // best effort cleanup _ = cfg.RemoveForkPoint(branch) //nolint:errcheck // best effort cleanup + _ = cfg.RemovePRBase(branch) //nolint:errcheck // best effort cleanup return mergedActionOrphan } diff --git a/cmd/undo.go b/cmd/undo.go index 76ac651..3d99613 100644 --- a/cmd/undo.go +++ b/cmd/undo.go @@ -36,9 +36,9 @@ var ( ) func init() { - rootCmd.AddCommand(undoCmd) undoCmd.Flags().BoolVarP(&undoForce, "force", "f", false, "Skip confirmation prompt") - undoCmd.Flags().BoolVar(&undoDryRun, "dry-run", false, "Show what would be restored without making changes") + undoCmd.Flags().BoolVarP(&undoDryRun, "dry-run", "D", false, "Show what would be restored without making changes") + rootCmd.AddCommand(undoCmd) } func runUndo(cmd *cobra.Command, args []string) error { @@ -52,7 +52,7 @@ func runUndo(cmd *cobra.Command, args []string) error { g := git.New(cwd) gitDir := g.GetGitDir() - // Check if a cascade/submit is in progress + // Check if a restack/submit is in progress if state.Exists(gitDir) { return errors.New("cannot undo while an operation is in progress; run 'gh stack continue' or 'gh stack abort' first") } diff --git a/cmd/unlink.go b/cmd/unlink.go index e5e6194..c19ed24 100644 --- a/cmd/unlink.go +++ b/cmd/unlink.go @@ -42,6 +42,7 @@ func runUnlink(cmd *cobra.Command, args []string) error { if err := cfg.RemovePR(branch); err != nil { return err } + _ = cfg.RemovePRBase(branch) //nolint:errcheck // best effort cleanup s := style.New() fmt.Printf("%s Unlinked PR from branch %s\n", s.SuccessIcon(), s.Branch(branch)) diff --git a/e2e/chaos_manual_git_test.go b/e2e/chaos_manual_git_test.go index f432aca..bdbf488 100644 --- a/e2e/chaos_manual_git_test.go +++ b/e2e/chaos_manual_git_test.go @@ -60,7 +60,7 @@ func TestManualRebase(t *testing.T) { env.Git("checkout", "main") env.CreateCommit("main moved") - // User rebases manually with git instead of cascade + // User rebases manually with git instead of restack env.Git("checkout", "feature-1") env.Git("rebase", "main") @@ -105,20 +105,20 @@ func TestManualBranchDelete(t *testing.T) { } } -func TestManualRebaseAbortDuringCascade(t *testing.T) { +func TestManualRebaseAbortDuringRestack(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") // Set up conflict scenario env.CreateStackWithConflict() - env.Run("cascade") // Will conflict and leave rebase in progress + env.Run("restack") // Will conflict and leave rebase in progress env.AssertRebaseInProgress() // User manually aborts with git instead of gh-stack abort env.Git("rebase", "--abort") - // gh-stack abort should still work (cleans up cascade state) + // gh-stack abort should still work (cleans up restack state) result := env.Run("abort") if result.Failed() { t.Errorf("abort should succeed after manual rebase --abort, got: %s", result.Stderr) diff --git a/e2e/chaos_remote_test.go b/e2e/chaos_remote_test.go index 5c5d027..b436d27 100644 --- a/e2e/chaos_remote_test.go +++ b/e2e/chaos_remote_test.go @@ -15,22 +15,22 @@ func TestRemoteTrunkAhead(t *testing.T) { // Simulate remote main moving ahead (another dev merged something) env.SimulateSomeoneElsePushed("main") - // Local doesn't know yet - cascade uses local main + // Local doesn't know yet - restack uses local main env.Git("checkout", "feature-1") - result := env.Run("cascade") + result := env.Run("restack") // Should succeed with local state (feature-1 already up-to-date with local main) if result.Failed() { - t.Errorf("cascade should work with local state: %s", result.Stderr) + t.Errorf("restack should work with local state: %s", result.Stderr) } // After fetch, local sees remote is ahead env.FetchOrigin() - // Now cascade would pick up the new main commits - result = env.Run("cascade") + // Now restack would pick up the new main commits + result = env.Run("restack") if result.Failed() { - t.Errorf("cascade after fetch should work: %s", result.Stderr) + t.Errorf("restack after fetch should work: %s", result.Stderr) } } @@ -47,7 +47,7 @@ func TestLocalTrunkAhead(t *testing.T) { // Don't push! env.Git("checkout", "feature-1") - env.MustRun("cascade") + env.MustRun("restack") // Should work with local main (includes unpushed commit) env.AssertAncestor("main", "feature-1") @@ -96,14 +96,14 @@ func TestSomeoneElsePushedToMyBranch(t *testing.T) { // Submit should fail - remote has diverged (--force-with-lease protects us) result := env.Run("submit", "--dry-run") - // In dry-run, cascade/push phases shown but no actual push + // In dry-run, restack/push phases shown but no actual push // The actual failure would happen on real push with --force-with-lease if result.Failed() { t.Logf("submit dry-run result: %s", result.Stderr) } } -func TestSubmitAfterCascade(t *testing.T) { +func TestSubmitAfterRestack(t *testing.T) { env := NewTestEnvWithRemote(t) env.MustRun("init") @@ -121,13 +121,13 @@ func TestSubmitAfterCascade(t *testing.T) { env.Git("checkout", "feature-1") result := env.MustRun("submit", "--dry-run") - // Should show cascade needed + // Should show restack needed if !result.ContainsStdout("Would rebase") { t.Error("submit should show rebase would happen") } } -func TestFetchBeforeCascade(t *testing.T) { +func TestFetchBeforeRestack(t *testing.T) { env := NewTestEnvWithRemote(t) env.MustRun("init") @@ -137,10 +137,10 @@ func TestFetchBeforeCascade(t *testing.T) { // Remote main moves env.SimulateSomeoneElsePushed("main") - // Without fetch, cascade uses stale local main - result := env.Run("cascade") - // Document: does cascade auto-fetch? Or use local state? + // Without fetch, restack uses stale local main + result := env.Run("restack") + // Document: does restack auto-fetch? Or use local state? if result.Failed() { - t.Errorf("cascade should not fail: %s", result.Stderr) + t.Errorf("restack should not fail: %s", result.Stderr) } } diff --git a/e2e/error_cases_test.go b/e2e/error_cases_test.go index be8ea83..afc4585 100644 --- a/e2e/error_cases_test.go +++ b/e2e/error_cases_test.go @@ -38,7 +38,7 @@ func TestCreateWithModifiedTrackedFile(t *testing.T) { env.AssertBranch("feature-1") } -func TestCascadeWithDirtyTree(t *testing.T) { +func TestRestackWithDirtyTree(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -52,11 +52,11 @@ func TestCascadeWithDirtyTree(t *testing.T) { // Make dirty with modified tracked file env.WriteFile("README.md", "modified content") - result := env.Run("cascade") + result := env.Run("restack") // Should auto-stash and succeed if result.Failed() { - t.Errorf("cascade should auto-stash and succeed, got: %s", result.Stderr) + t.Errorf("restack should auto-stash and succeed, got: %s", result.Stderr) } if !result.ContainsStdout("Auto-stashed") { t.Errorf("expected auto-stash message, got: %s", result.Stdout) diff --git a/e2e/orphan_test.go b/e2e/orphan_test.go new file mode 100644 index 0000000..50793fc --- /dev/null +++ b/e2e/orphan_test.go @@ -0,0 +1,55 @@ +// e2e/orphan_test.go +package e2e_test + +import "testing" + +func TestOrphanClearsPRBase(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.MustRun("create", "feature-a") + env.CreateCommit("feature a work") + + // Simulate a prior submit run that stored a PR base. + env.Git("config", "branch.feature-a.stackPR", "7") + env.Git("config", "branch.feature-a.stackPRBase", "main") + + // Verify both keys are present before orphan. + if env.GetStackConfig("branch.feature-a.stackPRBase") != "main" { + t.Fatal("stackPRBase should be set before orphan") + } + + env.Git("checkout", "main") + env.MustRun("orphan", "feature-a") + + if v := env.GetStackConfig("branch.feature-a.stackPR"); v != "" { + t.Errorf("expected stackPR to be removed after orphan, got %q", v) + } + if v := env.GetStackConfig("branch.feature-a.stackPRBase"); v != "" { + t.Errorf("expected stackPRBase to be removed after orphan, got %q", v) + } +} + +func TestOrphanForceClearsPRBaseOnDescendants(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + env.MustRun("create", "feat-b") + env.CreateCommit("b work") + + // Simulate stored PR bases on both branches. + env.Git("config", "branch.feat-a.stackPRBase", "main") + env.Git("config", "branch.feat-b.stackPRBase", "feat-a") + + env.Git("checkout", "main") + env.MustRun("orphan", "--force", "feat-a") + + if v := env.GetStackConfig("branch.feat-a.stackPRBase"); v != "" { + t.Errorf("expected feat-a stackPRBase cleared, got %q", v) + } + if v := env.GetStackConfig("branch.feat-b.stackPRBase"); v != "" { + t.Errorf("expected feat-b stackPRBase cleared, got %q", v) + } +} diff --git a/e2e/cascade_test.go b/e2e/restack_test.go similarity index 86% rename from e2e/cascade_test.go rename to e2e/restack_test.go index 1667de8..95595d2 100644 --- a/e2e/cascade_test.go +++ b/e2e/restack_test.go @@ -1,9 +1,9 @@ -// e2e/cascade_test.go +// e2e/restack_test.go package e2e_test import "testing" -func TestCascadeSimple(t *testing.T) { +func TestRestackSimple(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -18,9 +18,9 @@ func TestCascadeSimple(t *testing.T) { env.Git("checkout", "main") env.CreateCommit("main moved forward") - // Cascade from feature-a + // Restack from feature-a env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") // Verify ancestry env.AssertAncestor("main", "feature-a") @@ -28,7 +28,7 @@ func TestCascadeSimple(t *testing.T) { env.AssertNoRebaseInProgress() } -func TestCascadeDeepStack(t *testing.T) { +func TestRestackDeepStack(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -43,9 +43,9 @@ func TestCascadeDeepStack(t *testing.T) { env.Git("checkout", "main") env.CreateCommit("main update") - // Cascade from first branch + // Restack from first branch env.Git("checkout", "feat-1") - env.MustRun("cascade") + env.MustRun("restack") // Verify chain env.AssertAncestor("main", "feat-1") @@ -54,17 +54,17 @@ func TestCascadeDeepStack(t *testing.T) { } } -func TestCascadeWithConflict(t *testing.T) { +func TestRestackWithConflict(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") _ = env.CreateStackWithConflict() - result := env.Run("cascade") + result := env.Run("restack") - // Cascade returns non-zero on conflict (consistent with git rebase) + // Restack returns non-zero on conflict (consistent with git rebase) if result.Success() { - t.Error("cascade should return non-zero exit on conflict") + t.Error("restack should return non-zero exit on conflict") } if !result.ContainsStdout("CONFLICT") { t.Errorf("expected CONFLICT in output, got: %s", result.Stdout) @@ -72,15 +72,15 @@ func TestCascadeWithConflict(t *testing.T) { env.AssertRebaseInProgress() } -func TestCascadeAbort(t *testing.T) { +func TestRestackAbort(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") env.CreateStackWithConflict() - result := env.Run("cascade") + result := env.Run("restack") if result.Success() { - t.Fatal("expected cascade to fail on conflict") + t.Fatal("expected restack to fail on conflict") } env.AssertRebaseInProgress() @@ -89,15 +89,15 @@ func TestCascadeAbort(t *testing.T) { env.AssertNoRebaseInProgress() } -func TestCascadeContinue(t *testing.T) { +func TestRestackContinue(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") conflictFile := env.CreateStackWithConflict() - result := env.Run("cascade") + result := env.Run("restack") if result.Success() { - t.Fatal("expected cascade to fail on conflict") + t.Fatal("expected restack to fail on conflict") } env.AssertRebaseInProgress() @@ -108,7 +108,7 @@ func TestCascadeContinue(t *testing.T) { env.AssertNoRebaseInProgress() } -func TestCascadeReturnsToOriginalBranch(t *testing.T) { +func TestRestackReturnsToOriginalBranch(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -126,11 +126,11 @@ func TestCascadeReturnsToOriginalBranch(t *testing.T) { env.Git("checkout", "main") env.CreateCommit("main moved forward") - // Start cascade from feature-a (not the deepest branch) + // Start restack from feature-a (not the deepest branch) env.Git("checkout", "feature-a") env.AssertBranch("feature-a") - env.MustRun("cascade") + env.MustRun("restack") // Verify we returned to feature-a, not feature-c (the last restacked branch) env.AssertBranch("feature-a") @@ -141,7 +141,7 @@ func TestCascadeReturnsToOriginalBranch(t *testing.T) { env.AssertAncestor("feature-b", "feature-c") } -func TestCascadeStaleForkPointFromManualRebase(t *testing.T) { +func TestRestackStaleForkPointFromManualRebase(t *testing.T) { // Reproduces the bug where a manual rebase outside gh-stack leaves the // fork point stale. On the next restack after main advances, the stale // fork point would trigger an --onto rebase that replays too many commits. @@ -180,7 +180,7 @@ func TestCascadeStaleForkPointFromManualRebase(t *testing.T) { env.AssertNoRebaseInProgress() } -func TestCascadeStaleForkPointDetectedDuringRebase(t *testing.T) { +func TestRestackStaleForkPointDetectedDuringRebase(t *testing.T) { // Even if the "already up to date" refresh was missed, the ancestor check // in the useOnto logic should prevent --onto with a stale fork point. env := NewTestEnv(t) @@ -228,15 +228,15 @@ func TestCascadeStaleForkPointDetectedDuringRebase(t *testing.T) { env.AssertNoRebaseInProgress() } -func TestCascadeForkPointUpdatedAfterContinue(t *testing.T) { +func TestRestackForkPointUpdatedAfterContinue(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") // Create a stack that will conflict conflictFile := env.CreateStackWithConflict() - // Cascade will hit a conflict on feature-b - result := env.Run("cascade") + // Restack will hit a conflict on feature-b + result := env.Run("restack") if result.Success() { t.Fatal("expected conflict") } @@ -261,7 +261,7 @@ func TestCascadeForkPointUpdatedAfterContinue(t *testing.T) { } } -func TestCascadeOntoUsedForRewrittenParent(t *testing.T) { +func TestRestackOntoUsedForRewrittenParent(t *testing.T) { // Verify that --onto IS used when the parent's history was actually // rewritten (not just a stale fork point). env := NewTestEnv(t) diff --git a/e2e/scenario_helpers_test.go b/e2e/scenario_helpers_test.go index c4bd2f5..e9edc40 100644 --- a/e2e/scenario_helpers_test.go +++ b/e2e/scenario_helpers_test.go @@ -31,7 +31,7 @@ func (e *TestEnv) CreateConflict(baseBranch, featureBranch string) string { } // CreateStackWithConflict creates a 3-branch stack where cascading will conflict. -// After setup, you should be on feature-a and cascade from there. +// After setup, you should be on feature-a and restack from there. // The conflict occurs when feature-b rebases onto feature-a (after feature-a picks up main's changes). func (e *TestEnv) CreateStackWithConflict() string { e.t.Helper() @@ -61,7 +61,7 @@ func (e *TestEnv) CreateStackWithConflict() string { e.Git("add", conflictFile) e.Git("commit", "-m", "main: modify shared.txt") - // Return to feature-a for cascade (cascade operates on current + descendants) + // Return to feature-a for restack (restack operates on current + descendants) e.Git("checkout", "feature-a") return conflictFile diff --git a/e2e/submit_test.go b/e2e/submit_test.go index 74c2dfb..3225d07 100644 --- a/e2e/submit_test.go +++ b/e2e/submit_test.go @@ -76,8 +76,8 @@ func TestSubmitCurrentOnlyDryRun(t *testing.T) { env.Git("checkout", "feat-a") - // Submit --current-only should NOT cascade feat-b - result := env.MustRun("submit", "--dry-run", "--current-only") + // Submit --current should NOT restack feat-b + result := env.MustRun("submit", "--dry-run", "--current") // Should mention feat-a but not feat-b in push phase output := result.Stdout @@ -85,11 +85,11 @@ func TestSubmitCurrentOnlyDryRun(t *testing.T) { t.Error("expected feat-a push in output") } if strings.Contains(output, "Would push feat-b") { - t.Error("feat-b should not be pushed with --current-only") + t.Error("feat-b should not be pushed with --current") } } -func TestSubmitWithCascadeNeeded(t *testing.T) { +func TestSubmitWithRestackNeeded(t *testing.T) { env := NewTestEnvWithRemote(t) env.MustRun("init") @@ -109,7 +109,7 @@ func TestSubmitWithCascadeNeeded(t *testing.T) { // Should show rebase would happen if !strings.Contains(result.Stdout, "Would rebase feat-b onto feat-a") { - t.Error("expected cascade of feat-b onto feat-a") + t.Error("expected restack of feat-b onto feat-a") } } @@ -125,7 +125,7 @@ func TestSubmitAlreadyUpToDate(t *testing.T) { // Should indicate already up to date if !strings.Contains(result.Stdout, "already up to date") { - t.Error("expected 'already up to date' for branch that doesn't need cascade") + t.Error("expected 'already up to date' for branch that doesn't need restack") } } @@ -136,13 +136,13 @@ func TestSubmitUpdateOnlyDryRun(t *testing.T) { env.MustRun("create", "feature-1") env.CreateCommit("feature 1 work") - // With --update-only, should skip PR creation for branches without PRs - result := env.MustRun("submit", "--dry-run", "--update-only") + // With --update, should skip PR creation for branches without PRs + result := env.MustRun("submit", "--dry-run", "--update") - // Should not show "Would create PR" since update-only skips creation + // Should not show "Would create PR" since update skips creation // (In dry-run, it should show the skip message) if strings.Contains(result.Stdout, "Would create PR") { - t.Error("should not create PR with --update-only") + t.Error("should not create PR with --update") } } @@ -247,11 +247,11 @@ func TestSubmitFromTrunkCurrentOnlyFails(t *testing.T) { // Go back to trunk env.Git("checkout", "main") - // Submit --current-only from trunk should fail - result := env.Run("submit", "--dry-run", "--current-only") + // Submit --current from trunk should fail + result := env.Run("submit", "--dry-run", "--current") if result.Success() { - t.Error("expected submit --current-only from trunk to fail") + t.Error("expected submit --current from trunk to fail") } if !strings.Contains(result.Stderr, "cannot submit trunk") { t.Errorf("expected error about trunk, got: %s", result.Stderr) @@ -281,7 +281,7 @@ func TestSubmitPushOnlyDryRun(t *testing.T) { env.MustRun("create", "feature-1") env.CreateCommit("feature 1 work") - result := env.MustRun("submit", "--dry-run", "--push-only") + result := env.MustRun("submit", "--dry-run", "--skip-prs") // Should show all three phases if !strings.Contains(result.Stdout, "Phase 1: Restack") { @@ -295,13 +295,13 @@ func TestSubmitPushOnlyDryRun(t *testing.T) { } // Should show skipped message for PR phase - if !strings.Contains(result.Stdout, "Skipped (--push-only)") { - t.Error("expected 'Skipped (--push-only)' message") + if !strings.Contains(result.Stdout, "Skipped (--skip-prs)") { + t.Error("expected 'Skipped (--skip-prs)' message") } // Should NOT mention PR creation if strings.Contains(result.Stdout, "Would create PR") { - t.Error("should not mention PR creation with --push-only") + t.Error("should not mention PR creation with --skip-prs") } } @@ -312,12 +312,12 @@ func TestSubmitPushOnlyWithUpdateOnlyFails(t *testing.T) { env.MustRun("create", "feature-1") env.CreateCommit("feature 1 work") - result := env.Run("submit", "--dry-run", "--push-only", "--update-only") + result := env.Run("submit", "--dry-run", "--skip-prs", "--update") if result.Success() { - t.Error("expected --push-only and --update-only to fail") + t.Error("expected --skip-prs and --update to fail") } - if !strings.Contains(result.Stderr, "--push-only and --update-only cannot be used together") { + if !strings.Contains(result.Stderr, "--skip-prs and --update cannot be used together") { t.Errorf("expected error about conflicting flags, got: %s", result.Stderr) } } @@ -329,12 +329,12 @@ func TestSubmitPushOnlyWithWebFails(t *testing.T) { env.MustRun("create", "feature-1") env.CreateCommit("feature 1 work") - result := env.Run("submit", "--dry-run", "--push-only", "--web") + result := env.Run("submit", "--dry-run", "--skip-prs", "--web") if result.Success() { - t.Error("expected --push-only and --web to fail") + t.Error("expected --skip-prs and --web to fail") } - if !strings.Contains(result.Stderr, "--push-only and --web cannot be used together") { + if !strings.Contains(result.Stderr, "--skip-prs and --web cannot be used together") { t.Errorf("expected error about conflicting flags, got: %s", result.Stderr) } } @@ -456,12 +456,12 @@ func TestSubmitFromAndCurrentOnlyMutualExclusion(t *testing.T) { env.MustRun("create", "feat-a") env.CreateCommit("a work") - result := env.Run("submit", "--dry-run", "--from=feat-a", "--current-only") + result := env.Run("submit", "--dry-run", "--from=feat-a", "--current") if result.Success() { - t.Error("expected --from and --current-only to fail") + t.Error("expected --from and --current to fail") } - if !strings.Contains(result.Stderr, "--from and --current-only cannot be used together") { + if !strings.Contains(result.Stderr, "--from and --current cannot be used together") { t.Errorf("expected error about conflicting flags, got: %s", result.Stderr) } } diff --git a/e2e/sync_test.go b/e2e/sync_test.go index 42200c8..821bfc9 100644 --- a/e2e/sync_test.go +++ b/e2e/sync_test.go @@ -5,11 +5,11 @@ import "testing" // Note: Full E2E testing of sync's return-to-original-branch behavior is limited // because sync requires a real GitHub remote. The underlying return-to-branch logic -// is tested via TestCascadeReturnsToOriginalBranch in cascade_test.go, since sync -// delegates restacking to the same doCascadeWithState function. +// is tested via TestRestackReturnsToOriginalBranch in restack_test.go, since sync +// delegates restacking to the same doRestackWithState function. // // The fix for issue #58 (sync leaving user on wrong branch) is validated by: -// 1. TestCascadeReturnsToOriginalBranch - tests the shared restack infrastructure +// 1. TestRestackReturnsToOriginalBranch - tests the shared restack infrastructure // 2. cmd/sync_test.go unit tests - test sync-specific starting branch capture: // - TestSyncStartingBranchCapture - normal branch detection // - TestSyncStartingBranchDetachedHEAD - "HEAD" normalization for detached state diff --git a/e2e/undo_test.go b/e2e/undo_test.go index 9223b05..2f5b6e2 100644 --- a/e2e/undo_test.go +++ b/e2e/undo_test.go @@ -7,7 +7,7 @@ import ( "testing" ) -func TestUndoCascade(t *testing.T) { +func TestUndoRestack(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -24,9 +24,9 @@ func TestUndoCascade(t *testing.T) { env.Git("checkout", "main") env.CreateCommit("main moved forward") - // Cascade from feature-a + // Restack from feature-a env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") // Verify branches moved newFeatureASha := env.BranchTip("feature-a") @@ -38,7 +38,7 @@ func TestUndoCascade(t *testing.T) { t.Error("feature-b should have been rebased") } - // Undo the cascade + // Undo the restack env.MustRun("undo", "--force") // Verify branches restored @@ -56,7 +56,7 @@ func TestUndoDryRun(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") - // Create stack and cascade + // Create stack and restack env.MustRun("create", "feature-a") env.CreateCommit("feature a work") featureASha := env.BranchTip("feature-a") @@ -65,12 +65,12 @@ func TestUndoDryRun(t *testing.T) { env.CreateCommit("main moved forward") env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") // Verify branch moved - cascadedSha := env.BranchTip("feature-a") - if cascadedSha == featureASha { - t.Fatal("cascade didn't change SHA") + restackdSha := env.BranchTip("feature-a") + if restackdSha == featureASha { + t.Fatal("restack didn't change SHA") } // Dry run should not change anything @@ -79,9 +79,9 @@ func TestUndoDryRun(t *testing.T) { t.Error("expected dry-run message in output") } - // Branch should still be at cascaded SHA + // Branch should still be at restackd SHA afterDryRunSha := env.BranchTip("feature-a") - if afterDryRunSha != cascadedSha { + if afterDryRunSha != restackdSha { t.Error("dry-run should not have changed branch") } } @@ -109,9 +109,9 @@ func TestUndoRestoresConfig(t *testing.T) { env.CreateCommit("main moved forward") env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") - // Cascade updates fork point + // Restack updates fork point newForkPoint := env.GetStackConfig("branch.feature-a.stackforkpoint") if newForkPoint == originalForkPoint { // Fork point should have changed (or been set) @@ -139,7 +139,7 @@ func TestUndoArchivesSnapshot(t *testing.T) { env.CreateCommit("main moved forward") env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") // Verify snapshot exists undoDir := filepath.Join(env.WorkDir, ".git", "stack-undo") @@ -183,7 +183,7 @@ func TestUndoArchivesSnapshot(t *testing.T) { } } -func TestCascadeWithAutoStashRestoresAfterSuccess(t *testing.T) { +func TestRestackWithAutoStashRestoresAfterSuccess(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -198,19 +198,19 @@ func TestCascadeWithAutoStashRestoresAfterSuccess(t *testing.T) { // Create uncommitted changes env.WriteFile("uncommitted.txt", "uncommitted content\n") - // Cascade should auto-stash and then restore after success - result := env.MustRun("cascade") + // Restack should auto-stash and then restore after success + result := env.MustRun("restack") if !result.ContainsStdout("Auto-stashed") { t.Error("expected auto-stash message") } if !result.ContainsStdout("Restoring auto-stashed") { - t.Error("expected restore message after successful cascade") + t.Error("expected restore message after successful restack") } - // Uncommitted file should be restored after successful cascade + // Uncommitted file should be restored after successful restack content, err := os.ReadFile(filepath.Join(env.WorkDir, "uncommitted.txt")) if err != nil { - t.Errorf("uncommitted file should be present after cascade: %v", err) + t.Errorf("uncommitted file should be present after restack: %v", err) } else if string(content) != "uncommitted content\n" { t.Errorf("uncommitted file has wrong content: %q", content) } @@ -229,9 +229,9 @@ func TestUndoRestoresOriginalBranch(t *testing.T) { env.Git("checkout", "main") env.CreateCommit("main moved forward") - // Cascade from feature-a + // Restack from feature-a env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") // Should still be on feature-a env.AssertBranch("feature-a") @@ -239,39 +239,39 @@ func TestUndoRestoresOriginalBranch(t *testing.T) { // Now checkout something else env.Git("checkout", "feature-b") - // Undo should restore to feature-a (original head at time of cascade) + // Undo should restore to feature-a (original head at time of restack) env.MustRun("undo", "--force") env.AssertBranch("feature-a") } -func TestUndoBlockedDuringCascade(t *testing.T) { +func TestUndoBlockedDuringRestack(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") conflictFile := env.CreateStackWithConflict() _ = conflictFile - // Start cascade (will conflict) - result := env.Run("cascade") + // Start restack (will conflict) + result := env.Run("restack") if result.Success() { - t.Fatal("expected cascade to fail on conflict") + t.Fatal("expected restack to fail on conflict") } env.AssertRebaseInProgress() // Undo should be blocked result = env.Run("undo", "--force") if result.Success() { - t.Error("undo should fail during cascade in progress") + t.Error("undo should fail during restack in progress") } if !result.ContainsStdout("operation is in progress") && !result.ContainsStderr("operation is in progress") { t.Errorf("expected message about operation in progress, got stdout: %s, stderr: %s", result.Stdout, result.Stderr) } - // Abort the cascade + // Abort the restack env.MustRun("abort") } -func TestMultipleCascadesUndoLatest(t *testing.T) { +func TestMultipleRestacksUndoLatest(t *testing.T) { env := NewTestEnv(t) env.MustRun("init") @@ -280,33 +280,33 @@ func TestMultipleCascadesUndoLatest(t *testing.T) { env.CreateCommit("feature a v1") featureAv1 := env.BranchTip("feature-a") - // First cascade trigger + // First restack trigger env.Git("checkout", "main") env.CreateCommit("main update 1") env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") featureAAfterFirst := env.BranchTip("feature-a") if featureAAfterFirst == featureAv1 { - t.Fatal("first cascade didn't change SHA") + t.Fatal("first restack didn't change SHA") } - // Second cascade trigger + // Second restack trigger env.Git("checkout", "main") env.CreateCommit("main update 2") env.Git("checkout", "feature-a") - env.MustRun("cascade") + env.MustRun("restack") featureAAfterSecond := env.BranchTip("feature-a") if featureAAfterSecond == featureAAfterFirst { - t.Fatal("second cascade didn't change SHA") + t.Fatal("second restack didn't change SHA") } - // Undo should restore to state before SECOND cascade (not first) + // Undo should restore to state before SECOND restack (not first) env.MustRun("undo", "--force") featureAAfterUndo := env.BranchTip("feature-a") if featureAAfterUndo != featureAAfterFirst { - t.Errorf("undo should restore to state before second cascade: expected %s, got %s", + t.Errorf("undo should restore to state before second restack: expected %s, got %s", featureAAfterFirst, featureAAfterUndo) } } diff --git a/e2e/unlink_test.go b/e2e/unlink_test.go new file mode 100644 index 0000000..bbe1398 --- /dev/null +++ b/e2e/unlink_test.go @@ -0,0 +1,35 @@ +// e2e/unlink_test.go +package e2e_test + +import "testing" + +func TestUnlinkClearsPRBase(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.MustRun("create", "feature-a") + env.CreateCommit("feature a work") + + // Simulate a prior submit run that stored a PR base. + env.Git("config", "branch.feature-a.stackPR", "42") + env.Git("config", "branch.feature-a.stackPRBase", "main") + + // Verify both keys are present before unlink. + if env.GetStackConfig("branch.feature-a.stackPR") != "42" { + t.Fatal("stackPR should be set before unlink") + } + if env.GetStackConfig("branch.feature-a.stackPRBase") != "main" { + t.Fatal("stackPRBase should be set before unlink") + } + + env.Git("checkout", "feature-a") + env.MustRun("unlink") + + // Both PR-related keys should be cleared. + if v := env.GetStackConfig("branch.feature-a.stackPR"); v != "" { + t.Errorf("expected stackPR to be removed after unlink, got %q", v) + } + if v := env.GetStackConfig("branch.feature-a.stackPRBase"); v != "" { + t.Errorf("expected stackPRBase to be removed after unlink, got %q", v) + } +} diff --git a/e2e/worktree_test.go b/e2e/worktree_test.go index 6b8933f..f0ee38f 100644 --- a/e2e/worktree_test.go +++ b/e2e/worktree_test.go @@ -151,7 +151,7 @@ func TestRestackWithoutWorktreeFlagErrors(t *testing.T) { func TestSyncWithWorktree(t *testing.T) { // sync requires a real GitHub remote which we can't simulate in E2E tests. // Instead, verify the --worktrees flag is accepted by the sync command - // and test the cascade-with-worktrees behavior (which sync delegates to) + // and test the restack-with-worktrees behavior (which sync delegates to) // via the restack tests above. env := NewTestEnv(t) env.MustRun("init") diff --git a/internal/config/config.go b/internal/config/config.go index 4c93686..c42db5a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,6 +23,9 @@ var ErrNoPR = errors.New("no PR associated with branch") // ErrNoForkPoint is returned when a branch has no stored fork point. var ErrNoForkPoint = errors.New("no fork point stored for branch") +// ErrNoPRBase is returned when a branch has no stored last-known PR base. +var ErrNoPRBase = errors.New("no PR base stored for branch") + // Config provides access to stack metadata stored in .git/config. type Config struct { repoPath string @@ -123,6 +126,31 @@ func (c *Config) RemoveForkPoint(branch string) error { return nil } +// GetPRBase returns the last-known remote PR base branch for the given branch. +// This is used to detect when a PR's base transitions to trunk so the publish +// prompt fires only on that transition rather than on every submit run. +func (c *Config) GetPRBase(branch string) (string, error) { + key := "branch." + branch + ".stackPRBase" + out, err := exec.Command("git", "-C", c.repoPath, "config", "--get", key).Output() + if err != nil { + return "", ErrNoPRBase + } + return strings.TrimSpace(string(out)), nil +} + +// SetPRBase stores the last-known remote PR base branch for the given branch. +func (c *Config) SetPRBase(branch, base string) error { + key := "branch." + branch + ".stackPRBase" + return exec.Command("git", "-C", c.repoPath, "config", key, base).Run() +} + +// RemovePRBase removes the stored PR base for a branch. +func (c *Config) RemovePRBase(branch string) error { + key := "branch." + branch + ".stackPRBase" + _ = exec.Command("git", "-C", c.repoPath, "config", "--unset", key).Run() //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 diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9b395ca..31e585a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -155,6 +155,53 @@ func TestListTrackedBranches(t *testing.T) { } } +func TestPRBase(t *testing.T) { + dir := setupTestRepo(t) + cfg, _ := config.Load(dir) + + // Initially no PR base + _, err := cfg.GetPRBase("feature-a") + if !errors.Is(err, config.ErrNoPRBase) { + t.Errorf("GetPRBase = %v, want ErrNoPRBase", err) + } + + // Set PR base + if setErr := cfg.SetPRBase("feature-a", "main"); setErr != nil { + t.Fatalf("SetPRBase failed: %v", setErr) + } + + // Get PR base + got, err := cfg.GetPRBase("feature-a") + if err != nil { + t.Fatalf("GetPRBase failed: %v", err) + } + if got != "main" { + t.Errorf("GetPRBase = %q, want %q", got, "main") + } + + // Update PR base + if setErr := cfg.SetPRBase("feature-a", "feat-parent"); setErr != nil { + t.Fatalf("SetPRBase update failed: %v", setErr) + } + got, err = cfg.GetPRBase("feature-a") + if err != nil { + t.Fatalf("GetPRBase after update failed: %v", err) + } + if got != "feat-parent" { + t.Errorf("GetPRBase after update = %q, want %q", got, "feat-parent") + } + + // Remove PR base + if removeErr := cfg.RemovePRBase("feature-a"); removeErr != nil { + t.Fatalf("RemovePRBase failed: %v", removeErr) + } + + _, err = cfg.GetPRBase("feature-a") + if !errors.Is(err, config.ErrNoPRBase) { + t.Errorf("after remove, GetPRBase = %v, want ErrNoPRBase", err) + } +} + func TestForkPoint(t *testing.T) { dir := setupTestRepo(t) cfg, _ := config.Load(dir) diff --git a/internal/state/state.go b/internal/state/state.go index 39b9a0e..2caad10 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -10,21 +10,21 @@ import ( const stateFile = "STACK_CASCADE_STATE" -// Operation types for cascade state. +// Operation types for restack state. const ( - OperationCascade = "cascade" + OperationRestack = "restack" OperationSubmit = "submit" ) -// ErrNoState is returned when no cascade state exists. -var ErrNoState = errors.New("no cascade in progress") +// ErrNoState is returned when no restack state exists. +var ErrNoState = errors.New("no restack in progress") -// CascadeState represents the state of an in-progress cascade or submit operation. -type CascadeState struct { +// RestackState represents the state of an in-progress restack or submit operation. +type RestackState struct { Current string `json:"current"` Pending []string `json:"pending"` OriginalHead string `json:"original_head"` - // Operation is "cascade" or "submit" - determines what happens after cascade completes + // Operation is "restack" or "submit" - determines what happens after restack completes Operation string `json:"operation,omitempty"` // UpdateOnly (submit only) - if true, don't create new PRs, only update existing UpdateOnly bool `json:"update_only,omitempty"` @@ -33,7 +33,7 @@ type CascadeState struct { // PushOnly (submit only) - if true, skip PR creation/update phase entirely PushOnly bool `json:"push_only,omitempty"` // Branches (submit only) - the complete list of branches being submitted. - // Used to rebuild the full set for push/PR phases after cascade completes. + // Used to rebuild the full set for push/PR phases after restack completes. Branches []string `json:"branches,omitempty"` // StashRef is the commit hash of auto-stashed changes (if any). // Used to restore working tree changes when operation completes or is aborted. @@ -44,8 +44,8 @@ type CascadeState struct { Worktrees map[string]string `json:"worktrees,omitempty"` } -// Save persists cascade state to .git/STACK_CASCADE_STATE. -func Save(gitDir string, s *CascadeState) error { +// Save persists restack state to .git/STACK_CASCADE_STATE. +func Save(gitDir string, s *RestackState) error { path := filepath.Join(gitDir, stateFile) data, err := json.MarshalIndent(s, "", " ") if err != nil { @@ -54,8 +54,8 @@ func Save(gitDir string, s *CascadeState) error { return os.WriteFile(path, data, 0644) } -// Load reads cascade state from .git/STACK_CASCADE_STATE. -func Load(gitDir string) (*CascadeState, error) { +// Load reads restack state from .git/STACK_CASCADE_STATE. +func Load(gitDir string) (*RestackState, error) { path := filepath.Join(gitDir, stateFile) data, err := os.ReadFile(path) if err != nil { @@ -65,14 +65,14 @@ func Load(gitDir string) (*CascadeState, error) { return nil, err } - var s CascadeState + var s RestackState if err := json.Unmarshal(data, &s); err != nil { return nil, err } return &s, nil } -// Remove deletes the cascade state file. +// Remove deletes the restack state file. func Remove(gitDir string) error { path := filepath.Join(gitDir, stateFile) err := os.Remove(path) @@ -82,7 +82,7 @@ func Remove(gitDir string) error { return err } -// Exists checks if a cascade is in progress. +// Exists checks if a restack is in progress. func Exists(gitDir string) bool { path := filepath.Join(gitDir, stateFile) _, err := os.Stat(path) diff --git a/internal/state/state_test.go b/internal/state/state_test.go index d60fb53..6be9cc8 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -9,12 +9,12 @@ import ( "github.com/boneskull/gh-stack/internal/state" ) -func TestCascadeState(t *testing.T) { +func TestRestackState(t *testing.T) { dir := t.TempDir() gitDir := filepath.Join(dir, ".git") os.Mkdir(gitDir, 0755) - s := &state.CascadeState{ + s := &state.RestackState{ Current: "feature-b", Pending: []string{"feature-c", "feature-d"}, OriginalHead: "abc123", @@ -38,7 +38,7 @@ func TestCascadeState(t *testing.T) { } } -func TestCascadeStateNotExists(t *testing.T) { +func TestRestackStateNotExists(t *testing.T) { dir := t.TempDir() gitDir := filepath.Join(dir, ".git") os.Mkdir(gitDir, 0755) @@ -56,7 +56,7 @@ func TestSubmitState(t *testing.T) { t.Fatal(err) } - s := &state.CascadeState{ + s := &state.RestackState{ Current: "feature-b", Pending: []string{"feature-c"}, OriginalHead: "abc123", @@ -95,7 +95,7 @@ func TestSubmitStatePushOnly(t *testing.T) { t.Fatal(err) } - s := &state.CascadeState{ + s := &state.RestackState{ Current: "feature-b", Pending: []string{"feature-c"}, OriginalHead: "abc123", diff --git a/internal/undo/undo_test.go b/internal/undo/undo_test.go index fcfbc76..ce27da0 100644 --- a/internal/undo/undo_test.go +++ b/internal/undo/undo_test.go @@ -18,7 +18,7 @@ func TestSaveAndLoadLatest(t *testing.T) { t.Fatal(err) } - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "feature-a") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "feature-a") snapshot.Branches["feature-a"] = undo.BranchState{ SHA: "abc123def456", StackParent: "main", @@ -39,11 +39,11 @@ func TestSaveAndLoadLatest(t *testing.T) { t.Fatalf("LoadLatest failed: %v", err) } - if loaded.Operation != "cascade" { - t.Errorf("Operation mismatch: %q != %q", loaded.Operation, "cascade") + if loaded.Operation != "restack" { + t.Errorf("Operation mismatch: %q != %q", loaded.Operation, "restack") } - if loaded.Command != "gh stack cascade" { - t.Errorf("Command mismatch: %q != %q", loaded.Command, "gh stack cascade") + if loaded.Command != "gh stack restack" { + t.Errorf("Command mismatch: %q != %q", loaded.Command, "gh stack restack") } if loaded.OriginalHead != "feature-a" { t.Errorf("OriginalHead mismatch: %q != %q", loaded.OriginalHead, "feature-a") @@ -79,7 +79,7 @@ func TestLoadLatestReturnsNewest(t *testing.T) { } // Create first snapshot - snapshot1 := undo.NewSnapshot("cascade", "gh stack cascade", "feature-a") + snapshot1 := undo.NewSnapshot("restack", "gh stack restack", "feature-a") snapshot1.Timestamp = time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC) snapshot1.Branches["feature-a"] = undo.BranchState{SHA: "first"} if err := undo.Save(gitDir, snapshot1); err != nil { @@ -125,7 +125,7 @@ func TestArchive(t *testing.T) { t.Fatal(err) } - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "main") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "main") snapshot.Branches["feature"] = undo.BranchState{SHA: "abc123"} if err := undo.Save(gitDir, snapshot); err != nil { t.Fatal(err) @@ -172,7 +172,7 @@ func TestList(t *testing.T) { } // Create multiple snapshots - for i, op := range []string{"cascade", "submit", "sync"} { + for i, op := range []string{"restack", "submit", "sync"} { snapshot := undo.NewSnapshot(op, "gh stack "+op, "main") snapshot.Timestamp = time.Date(2024, 1, i+1, 12, 0, 0, 0, time.UTC) snapshot.Branches["feature"] = undo.BranchState{SHA: "abc"} @@ -194,8 +194,8 @@ func TestList(t *testing.T) { if snapshots[0].Operation != "sync" { t.Errorf("Expected sync (newest) first, got %q", snapshots[0].Operation) } - if snapshots[2].Operation != "cascade" { - t.Errorf("Expected cascade (oldest) last, got %q", snapshots[2].Operation) + if snapshots[2].Operation != "restack" { + t.Errorf("Expected restack (oldest) last, got %q", snapshots[2].Operation) } } @@ -210,7 +210,7 @@ func TestExists(t *testing.T) { t.Error("Exists should return false when no snapshots") } - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "main") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "main") snapshot.Branches["feature"] = undo.BranchState{SHA: "abc"} if err := undo.Save(gitDir, snapshot); err != nil { t.Fatal(err) @@ -272,7 +272,7 @@ func TestStashRef(t *testing.T) { t.Fatal(err) } - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "main") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "main") snapshot.StashRef = "stash@{0}" snapshot.Branches["feature"] = undo.BranchState{SHA: "abc"} @@ -297,7 +297,7 @@ func TestRemove(t *testing.T) { t.Fatal(err) } - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "main") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "main") snapshot.Branches["feature"] = undo.BranchState{SHA: "abc"} if err := undo.Save(gitDir, snapshot); err != nil { t.Fatal(err) @@ -331,7 +331,7 @@ func TestSavePrunesOldSnapshots(t *testing.T) { // Create 55 snapshots (exceeds the 50 limit) for i := range 55 { - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "main") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "main") // Use distinct timestamps to ensure unique filenames snapshot.Timestamp = time.Date(2024, 1, 1, 0, 0, i, 0, time.UTC) snapshot.Branches["feature"] = undo.BranchState{SHA: "abc"} @@ -373,7 +373,7 @@ func TestArchivePrunesOldSnapshots(t *testing.T) { // Create and archive 55 snapshots (exceeds the 50 limit) for i := range 55 { - snapshot := undo.NewSnapshot("cascade", "gh stack cascade", "main") + snapshot := undo.NewSnapshot("restack", "gh stack restack", "main") snapshot.Timestamp = time.Date(2024, 1, 1, 0, 0, i, 0, time.UTC) snapshot.Branches["feature"] = undo.BranchState{SHA: "abc"} if err := undo.Save(gitDir, snapshot); err != nil {