From bfbe4b83fea9a934596985bdb3c3142ba3be3203 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Wed, 28 Oct 2026 14:20:59 +0000 Subject: [PATCH] Sync before submit when a PR in the stack has merged Running git ss after the PR below had merged (but before a sync) pushed the branch as it was: gh stack skipped the merged branch and based the new PR on main, so it showed the merged commits as well as its own, and our result claimed we'd "created" the merged PR too. The merged flag in the metadata is only ever set by gh stack, so submit didn't know; the fresh PR list it fetches anyway did. So submit now treats a branch as merged when the forge says so, and when any branch of the stack has merged it runs the normal sync first (merged branch deleted, the ones above restacked onto trunk) and carries on from the stack as it is afterwards. If the merged branch was the one checked out, a whole stack submit picks up from whatever is left; a --down submit to it has nothing to do and says so. The sync is in the result (and shown by the CLI), and a dry run just names the branches and leaves them out of the plan. --- README.md | 5 ++ cmd/submit.go | 24 ++++++ docs/architecture.md | 14 +++ docs/mcp.md | 2 +- pkg/app/submit.go | 167 ++++++++++++++++++++++++++++-------- pkg/app/submit_base_test.go | 16 ++-- pkg/app/submit_sync_test.go | 117 +++++++++++++++++++++++++ pkg/mcp/instructions.go | 2 +- pkg/mcp/tools.go | 7 +- 9 files changed, 309 insertions(+), 45 deletions(-) create mode 100644 pkg/app/submit_sync_test.go diff --git a/README.md b/README.md index cd6a21a..fe38163 100644 --- a/README.md +++ b/README.md @@ -539,6 +539,11 @@ gh stack can only submit a whole stack, so with `--down` git-stack pushes those creates their missing PRs and has `gh stack link` add them to the stack on GitHub; the editor isn't available there, so new PRs take their titles from their commits unless you pass `--ai`. +If a PR lower in the stack has merged since you last synced, submit runs `git stack sync` first, +so the merged branch goes and the ones above it are restacked onto trunk before anything is pushed; +otherwise the next PR up would show the merged commits too. A dry run only says which branches that +would be. + Branches are pushed with `--force-with-lease`, so after a restack your branches are force pushed. If a branch has commits on the remote that aren't in your local one (someone else pushed to it), submit refuses rather than overwrite them. `git sync` pulls them in when the remote is diff --git a/cmd/submit.go b/cmd/submit.go index fac82b5..86137b3 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -38,6 +38,11 @@ we push those branches ourselves and have gh stack link add their PRs to the stack; the editor isn't available there, so new PRs take their titles from their commits unless you pass --ai. +If a PR lower in the stack has merged since you last synced, submit runs +git stack sync first, so the merged branch goes and the ones above it are +restacked onto trunk before anything is pushed; otherwise the next PR up would +show the merged commits too. A dry run only says which branches that would be. + Branches are pushed with --force-with-lease, so after a restack your branches are force pushed. If a branch has commits on the remote that aren't in your local one (someone else pushed to it), submit refuses rather than @@ -83,6 +88,16 @@ really do want them gone.`, opts.Branch = args[0] } res, err := a.Submit(ctx, repo, opts) + if res.Sync != nil { + rep.Warn("%s, so the stack was synced first", mergedPhrase(res.Merged)) + if err != nil { + // The error owns the summary line; what the sync did + // manage is still worth seeing. + renderSync(rep, *res.Sync) + return err + } + _ = reportSync(rep, *res.Sync, nil) + } if err != nil { return err } @@ -147,3 +162,12 @@ really do want them gone.`, cmd.Flags().BoolVar(&noAI, "no-ai", false, "never use AI; takes precedence over --ai and stack.ai.auto") return cmd } + +// mergedPhrase says whose pull requests have merged: "a's pull request has +// merged", "the pull requests of a and b have merged". +func mergedPhrase(names []string) string { + if len(names) == 1 { + return names[0] + "'s pull request has merged" + } + return "the pull requests of " + joinAnd(names) + " have merged" +} diff --git a/docs/architecture.md b/docs/architecture.md index e09f616..7d46ee3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -258,6 +258,20 @@ squash merge further down the remote still holds the parent's old commits, which anything locally by patch id. If the reflog has expired we fall back to just the patch id check, so the worst case is a refusal that needs `--force`, never a silent overwrite. +**Submit syncs first when a PR of the stack has merged.** The metadata's `merged` flag is +gh stack's, set when its submit checks the stack, so between a merge on GitHub and the next +`git stack sync` the local stack still has the merged branch with the others sitting on its +commits. Left to gh stack, submit skipped the merged branch and based the next PR on trunk, which +then showed the merged commits as well as its own. So submit now reads merged from the fresh PR +list it fetches anyway and, when any branch of the stack has merged, runs the normal sync before +touching the remote: the merged branch is deleted, the branches above are restacked onto trunk, +and the submit carries on from the stack as it is afterwards (from any surviving branch when the +current one was the merged one; a `--down` submit to it has nothing left and says so). The sync is +recorded for undo like any other, until the submit succeeds and clears the history. A dry run +can't sync, so it names the branches and leaves them out of the plan. A merged branch sync keeps +(prune `never`, a dirty checkout, merged into a parent that hasn't reached trunk) is skipped the +way gh stack skips it. + **`log` shows "needs push"** when a branch's remote tracking ref exists and the local branch has moved on from it (new commits, a restack, an amend). It's worked out from local refs only, so it's as fresh as the last fetch or push, and a remote that's simply ahead of the diff --git a/docs/mcp.md b/docs/mcp.md index eb2c2b5..50200da 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -35,7 +35,7 @@ path is resolved with `git rev-parse`; anything outside a repository returns `no | `stack_undo` | Undo the newest command run in this worktree that changed local state (create, modify, absorb, restack, reorder, move, fold, pop, untrack, init, sync, a local rename); call again to step further back, up to 20. After create, modify or absorb the changes come back staged (`staged: true`). A successful `stack_submit` or `stack_merge` clears the history. Refuses with `invalid_args` when nothing is left, when a branch moved since (`force` throws that away; uncommitted changes always stay) or when local changes are in the way, with nothing changed | `list` (the history with what each would put back, changing nothing), `force` (only with the user's say so) | destructive | | `stack_restack` | Local rebase of the stack onto its parents, bottom branch onto the local trunk included; only checks out a branch to resolve a conflict through one git rebase | `scope` = `all`/`upstack`/`downstack`/`only`, `branch`, `continue`, `abort`, `stage_all` | destructive | | `stack_navigate` | `up`/`down`/`top`/`bottom` with `steps`, or `branch` | `direction`, `steps`, `branch` | idempotent | -| `stack_submit` | Push and create/update chained PRs (with `down`, only up to the current branch); ready for review by default | `down`, `branch`, `draft`, `publish`, `dry_run`, `pull_requests` `{branch: {title, body}}`, `use_ai`, `force` | destructive, open-world | +| `stack_submit` | Push and create/update chained PRs (with `down`, only up to the current branch); ready for review by default; when a PR of the stack has merged since the last sync it syncs first (`merged` names the branches, `sync` says what happened) | `down`, `branch`, `draft`, `publish`, `dry_run`, `pull_requests` `{branch: {title, body}}`, `use_ai`, `force` | destructive, open-world | | `stack_sync` | Fetch, update trunk and restack every stack (checked out or not, all worktrees), never pushing; merged branches deleted per `stack.sync.prune` | `prune` (force deletion) | destructive, open-world | | `stack_merge` | Merge the stack's PRs up to a branch into trunk, all or nothing, then sync; refuses while a check on any of them failed or is still running | `branch` (default current), `method` = `merge`/`squash`/`rebase`, `no_sync`, `force` (skip the checks check; only with the user's say so) | destructive, open-world | diff --git a/pkg/app/submit.go b/pkg/app/submit.go index effb220..d787ade 100644 --- a/pkg/app/submit.go +++ b/pkg/app/submit.go @@ -73,8 +73,17 @@ type SubmitResult struct { Output string `json:"output,omitempty"` // Closed are the pull requests a pop or fold left behind that this // submit closed. - Closed []ClosedPR `json:"closed,omitempty"` - Notices []string `json:"notices,omitempty"` + Closed []ClosedPR `json:"closed,omitempty"` + // Merged names the branches of the stack whose pull requests had merged + // before this submit. Pushing over them would put their commits into + // the next PR up, so submit synced first (Sync says what that did): they + // went and the branches above were restacked onto trunk. A dry run only + // names them and leaves them out of the plan. + Merged []string `json:"merged,omitempty"` + // Sync is what the sync before the submit did, when one ran. It is set + // even when that sync failed, so what it did manage is still known. + Sync *SyncResult `json:"sync,omitempty"` + Notices []string `json:"notices,omitempty"` } // ClosedPR is a queued pull request a submit closed. @@ -131,38 +140,60 @@ func (a *App) submit(ctx context.Context, repo git.Repo, o SubmitOptions) (Submi } return SubmitResult{}, stack.Newf(stack.KindNotInStack, "%s is not in a stack", target) } + res := SubmitResult{DryRun: o.DryRun} + + // Existing PRs decide which branches are new, and which have merged + // since the last sync: the metadata only learns that from gh stack. + known, err := a.loadPRs(ctx, repo, PRsFresh) + if err != nil { + return SubmitResult{}, err + } + before := PRsFor(known) + for _, b := range s.Branches { + if mergedPR(before, b) { + res.Merged = append(res.Merged, b.Name) + } + } + if len(res.Merged) > 0 { + if o.DryRun { + res.Notices = append(res.Notices, fmt.Sprintf("%s; a real submit syncs first, deleting %s and restacking the ones above onto %s", + mergedPhrase(res.Merged), theBranches(res.Merged), s.Trunk)) + } else { + // A merged branch's commits are in trunk now, in some form, but + // still below the branches above it, and would show up in their + // pull requests. Sync deletes it and restacks them onto trunk. + s, idx, known, err = a.syncMerged(ctx, repo, target, s, o, &res) + if err != nil { + return res, err + } + before = PRsFor(known) + } + } + res.Stack = s.Names() + scope := s.Branches if o.Down || o.Branch != "" { scope = s.Branches[:idx+1] } - partial := slices.ContainsFunc(s.Branches[len(scope):], func(b stack.Branch) bool { return !b.Merged() }) - if !slices.ContainsFunc(scope, func(b stack.Branch) bool { return !b.Merged() }) { - return SubmitResult{}, stack.Newf(stack.KindInvalidArgs, "nothing to submit up to %s; its pull request has merged", target) + live := func(bs []stack.Branch) []stack.Branch { + return slices.DeleteFunc(slices.Clone(bs), func(b stack.Branch) bool { return mergedPR(before, b) }) + } + partial := len(live(s.Branches[len(scope):])) > 0 + if len(live(scope)) == 0 { + return res, stack.Newf(stack.KindInvalidArgs, "nothing to submit up to %s; its pull request has merged", target) } - res := SubmitResult{Stack: s.Names(), DryRun: o.DryRun} if !o.Force { - var live []string - for _, b := range scope { - if !b.Merged() { - live = append(live, b.Name) - } + var names []string + for _, b := range live(scope) { + names = append(names, b.Name) } - if err := a.checkPushedOver(ctx, repo, live); err != nil { - return SubmitResult{}, err + if err := a.checkPushedOver(ctx, repo, names); err != nil { + return res, err } } - // Existing PRs decide which branches are new. - known, err := a.loadPRs(ctx, repo, PRsFresh) - if err != nil { - return SubmitResult{}, err - } - before := PRsFor(known) var newBranches []string - for _, b := range scope { - if b.Merged() { - continue - } + for _, b := range live(scope) { pr, has := before[b.Name] entry := SubmittedPR{Branch: b.Name} if has && (pr.State == forge.StateOpen || pr.State == forge.StateDraft) { @@ -209,7 +240,7 @@ func (a *App) submit(ctx context.Context, repo git.Repo, o SubmitOptions) (Submi if editor { res.Notices = append(res.Notices, "gh stack's editor only opens for a whole stack submit; new PRs take their titles from their commits") } - if err := a.submitPart(ctx, repo, s, scope, texts, draft, &res); err != nil { + if err := a.submitPart(ctx, repo, s, live(scope), texts, draft, &res); err != nil { return SubmitResult{}, err } } else { @@ -295,18 +326,17 @@ func (a *App) submit(ctx context.Context, repo git.Repo, o SubmitOptions) (Submi } // submitPart submits only the branches in scope, a run of the stack from -// the bottom up. gh stack submit always pushes every branch, so this pushes -// the branches itself, creates their missing pull requests through the forge, -// and has gh stack link add the pull requests to the stack on GitHub. +// the bottom up with the merged ones left out. gh stack submit always pushes +// every branch, so this pushes the branches itself, creates their missing +// pull requests through the forge, and has gh stack link add the pull +// requests to the stack on GitHub. func (a *App) submitPart(ctx context.Context, repo git.Repo, s *stack.Stack, scope []stack.Branch, texts map[string]PRText, draft bool, res *SubmitResult) error { if a.d.Forge == nil { return ErrNoForge } - var live []string - for _, b := range scope { - if !b.Merged() { - live = append(live, b.Name) - } + live := make([]string, len(scope)) + for i, b := range scope { + live[i] = b.Name } top := live[len(live)-1] err := a.progress(ctx, PhaseSubmit, fmt.Sprintf("Submitting %s and the branches below it", top), func(ctx context.Context) error { @@ -352,6 +382,73 @@ func (a *App) submitPart(ctx context.Context, repo git.Repo, s *stack.Stack, sco return err } +// mergedPR reports whether a branch's pull request has merged, by the +// metadata or by what the forge said in prs. +func mergedPR(prs map[string]forge.PullRequest, b stack.Branch) bool { + pr, has := prs[b.Name] + return b.Merged() || (has && pr.State == forge.StateMerged) +} + +// mergedPhrase says whose pull requests have merged: "a's pull request has +// merged", "the pull requests of a and b have merged". +func mergedPhrase(names []string) string { + if len(names) == 1 { + return names[0] + "'s pull request has merged" + } + return "the pull requests of " + joinAnd(names) + " have merged" +} + +// joinAnd lists names as "a", "a and b" or "a, b and c". +func joinAnd(names []string) string { + if len(names) < 2 { + return strings.Join(names, "") + } + return strings.Join(names[:len(names)-1], ", ") + " and " + names[len(names)-1] +} + +// syncMerged runs the sync a submit needs when pull requests of its stack +// have merged, then finds the stack again: the target branch may have been +// the one that merged, in which case a whole stack submit carries on from +// any branch of the stack that is left, and a submit down to it has nothing +// left to do. Returns the stack, the target's index in it and the pull +// requests as they are afterwards. res.Sync is set whatever the sync did. +func (a *App) syncMerged(ctx context.Context, repo git.Repo, target string, s *stack.Stack, o SubmitOptions, res *SubmitResult) (*stack.Stack, int, []forge.PullRequest, error) { + sync, err := a.Sync(ctx, repo, SyncOptions{}) + res.Sync = &sync + if err != nil { + return nil, 0, nil, err + } + graph, err := a.d.Meta.Load(ctx, repo) + if err != nil { + return nil, 0, nil, err + } + after, idx, ok := graph.StackOf(target) + if !ok && !o.Down && o.Branch == "" { + for _, b := range s.Branches { + if slices.Contains(res.Merged, b.Name) { + continue + } + if after, idx, ok = graph.StackOf(b.Name); ok { + break + } + } + } + if !ok { + return nil, 0, nil, stack.Newf(stack.KindInvalidArgs, "nothing to submit: %s, so sync deleted %s", mergedPhrase(res.Merged), theBranches(res.Merged)). + WithSteps("run git stack log to see what is left") + } + known, err := a.loadPRs(ctx, repo, PRsFresh) + if err != nil { + return nil, 0, nil, err + } + return after, idx, known, nil +} + +// theBranches is "the branch" or "the branches". +func theBranches(names []string) string { + return pluralise(len(names), "the branch", "the branches") +} + // linkPart adds the submitted pull requests to the stack on GitHub. A single // pull request with no stack yet has nothing to join. A failure is only a // notice: the pull requests exist, and the next submit links them again. @@ -402,10 +499,6 @@ func (a *App) fixBases(ctx context.Context, repo git.Repo, s *stack.Stack, after if a.d.Forge == nil { return false } - merged := func(b stack.Branch) bool { - pr, has := after[b.Name] - return b.Merged() || (has && pr.State == forge.StateMerged) - } fixed := false for i := range res.PullRequests { e := &res.PullRequests[i] @@ -419,7 +512,7 @@ func (a *App) fixBases(ctx context.Context, repo git.Repo, s *stack.Stack, after } want := s.Trunk for j := idx - 1; j >= 0; j-- { - if !merged(s.Branches[j]) { + if !mergedPR(after, s.Branches[j]) { want = s.Branches[j].Name break } diff --git a/pkg/app/submit_base_test.go b/pkg/app/submit_base_test.go index 62f5126..90e98b0 100644 --- a/pkg/app/submit_base_test.go +++ b/pkg/app/submit_base_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/DomBlack/git-stack/pkg/app" - "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/config" ) // TestSubmitFixesBaseWhenGhStackSkipsQueuedBranches: gh stack bases a new @@ -37,14 +37,20 @@ func TestSubmitFixesBaseWhenGhStackSkipsQueuedBranches(t *testing.T) { t.Errorf("notice missing: %v", res.Notices) } - // When the branch below really has merged, trunk is the right base. - deps2, sf2, fg2, repo2, _ := submitFixture(t) + // When the branch below really has merged and sync kept it (the prune + // policy says never), trunk is the right base. + f2, sf2 := submitSyncFixture(t) + f2.deps.Config.SyncPrune = config.SyncPruneNever sf2.baseOnTrunk = true - fg2.prs[0].State = forge.StateMerged - res2, err := app.New(deps2).Submit(context.Background(), repo2, app.SubmitOptions{NoEdit: true}) + f2.mergeOnRemote(t, "a", 0) + fg2 := f2.forge + res2, err := app.New(f2.deps).Submit(context.Background(), f2.repo, app.SubmitOptions{NoEdit: true}) if err != nil { t.Fatal(err) } + if res2.Sync == nil || len(res2.Sync.Deleted) != 0 || len(res2.PullRequests) != 1 || res2.PullRequests[0].Branch != "b" { + t.Fatalf("a should have been kept by the sync and left out of the submit: %+v", res2) + } for _, pr := range res2.PullRequests { if pr.BaseFixed { t.Errorf("no base fix expected when the parent merged: %+v", pr) diff --git a/pkg/app/submit_sync_test.go b/pkg/app/submit_sync_test.go new file mode 100644 index 0000000..2921700 --- /dev/null +++ b/pkg/app/submit_sync_test.go @@ -0,0 +1,117 @@ +package app_test + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/git/gittest" + "github.com/DomBlack/git-stack/pkg/stack" +) + +// submitSyncFixture is the sync fixture (main -> a -> b with a bare origin, +// a's PR open, HEAD on b) with a submit backend, so a submit can sync. +func submitSyncFixture(t *testing.T) (*syncFixture, *submitFake) { + t.Helper() + f := newSyncFixture(t) + sf := &submitFake{forge: f.forge, graph: f.meta.g} + f.deps.Submit = sf + return f, sf +} + +// A pull request lower in the stack merged since the last sync, and the +// metadata doesn't know yet. Pushing as is would open the new PR with the +// merged branch's commits still in it, so submit syncs first: the merged +// branch goes, the one above lands on trunk, and that is what gets submitted. +func TestSubmitSyncsFirstWhenABranchBelowHasMerged(t *testing.T) { + f, sf := submitSyncFixture(t) + merge := f.mergeOnRemote(t, "a", 0) + res, err := app.New(f.deps).Submit(context.Background(), f.repo, app.SubmitOptions{NoEdit: true, Publish: true}) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(res.Merged, []string{"a"}) { + t.Errorf("merged = %v, want [a]", res.Merged) + } + if res.Sync == nil { + t.Fatal("submit should have synced first") + } + if d, ok := deleted(*res.Sync, "a"); !ok || d.Reason != app.ReasonMerged || branchExists(t, f.dir, "a") { + t.Errorf("a should have been deleted as merged: %+v", res.Sync.Deleted) + } + if got := f.rev(t, "b^"); got != merge { + t.Errorf("b should sit on the merge commit %s, is on %s", merge, got) + } + if !slices.Equal(res.Stack, []string{"b"}) { + t.Errorf("stack = %v, want [b]", res.Stack) + } + if len(res.PullRequests) != 1 || res.PullRequests[0].Branch != "b" || !res.PullRequests[0].Created { + t.Errorf("pull requests = %+v, want b created", res.PullRequests) + } + if len(sf.opts) != 1 { + t.Errorf("gh stack submit should have run once, ran %d times", len(sf.opts)) + } + pr := app.PRsFor(f.forge.prs)["b"] + if pr.Base != "main" { + t.Errorf("b's PR is based on %s, want main", pr.Base) + } +} + +// A dry run changes nothing, so it can't sync; it says which branches a +// real submit would sync away and leaves them out of the plan. +func TestSubmitDryRunReportsMergedBranchesWithoutSyncing(t *testing.T) { + f, sf := submitSyncFixture(t) + f.mergeOnRemote(t, "a", 0) + res, err := app.New(f.deps).Submit(context.Background(), f.repo, app.SubmitOptions{DryRun: true}) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(res.Merged, []string{"a"}) || res.Sync != nil || len(sf.opts) != 0 { + t.Errorf("dry run must report a as merged and not sync: %+v", res) + } + if !branchExists(t, f.dir, "a") { + t.Error("a dry run must not delete anything") + } + if len(res.PullRequests) != 1 || res.PullRequests[0].Branch != "b" || !res.PullRequests[0].WouldCreate { + t.Errorf("plan = %+v, want only b", res.PullRequests) + } + if n := strings.Join(res.Notices, "\n"); !strings.Contains(n, "a's pull request has merged") { + t.Errorf("notices = %v", res.Notices) + } +} + +// Submitting from the merged branch itself still submits the rest of the +// stack: sync moves the checkout to trunk and the branches above carry on. +func TestSubmitFromAMergedBranchSubmitsWhatIsLeft(t *testing.T) { + f, _ := submitSyncFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "a") + f.mergeOnRemote(t, "a", 0) + res, err := app.New(f.deps).Submit(context.Background(), f.repo, app.SubmitOptions{NoEdit: true, Publish: true}) + if err != nil { + t.Fatal(err) + } + if branchExists(t, f.dir, "a") || gittest.Run(t, f.dir, "branch", "--show-current") != "main" { + t.Error("a should be gone and the checkout on main") + } + if len(res.PullRequests) != 1 || res.PullRequests[0].Branch != "b" || !res.PullRequests[0].Created { + t.Errorf("pull requests = %+v, want b created", res.PullRequests) + } +} + +// With --down the merged branch is the top of what was asked for, so once +// sync has deleted it there is nothing left to submit. +func TestSubmitDownOnAMergedBranchHasNothingLeft(t *testing.T) { + f, sf := submitSyncFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "a") + f.mergeOnRemote(t, "a", 0) + res, err := app.New(f.deps).Submit(context.Background(), f.repo, app.SubmitOptions{NoEdit: true, Down: true}) + if se, ok := errors.AsType[*stack.Error](err); !ok || se.Kind != stack.KindInvalidArgs || !strings.Contains(se.Msg, "nothing to submit") { + t.Fatalf("err = %v, want nothing to submit", err) + } + if res.Sync == nil || len(sf.opts) != 0 { + t.Errorf("the sync should be reported and nothing pushed: %+v", res) + } +} diff --git a/pkg/mcp/instructions.go b/pkg/mcp/instructions.go index d0c61d6..405717e 100644 --- a/pkg/mcp/instructions.go +++ b/pkg/mcp/instructions.go @@ -2,7 +2,7 @@ package mcp // Instructions is sent to clients at initialisation. Codex weighs the start // of it most, so the key guidance comes first. -const Instructions = `git-stack manages stacked branches and stacked pull requests on top of GitHub's gh stack. Prefer these tools over raw git whenever the user works in a stack: stack_view shows every stack (branches, parents, PR state, needs_restack); stack_create starts a new branch on top of the current one (you write the commit message), or with insert: true right above the current branch in the middle of its stack, restacking the branches above onto it; stack_modify amends the current branch (or adds a commit) and restacks everything above it; stack_absorb folds each staged hunk into the commit of the stack that last changed those lines and rewrites that commit and everything above it (dry_run shows the plan first); use it to fix something lower down without navigating there; stack_reorder changes the order of the current stack's branches (order, bottom to top) and restacks them; stack_move puts a branch and what's above it on top of another stack or on a trunk as a new stack (only: true leaves the branches above behind); stack_fold merges the current branch into the branch below it, one branch and one pull request (keep: true keeps the current branch's name and pull request instead of the parent's), and the next stack_submit closes the pull request of the branch that went; stack_rename gives the current branch a new name, renaming it on GitHub first once it has been pushed so its pull request follows (local: true renames it only in this repository); stack_init tracks branches you already have as a new stack, without rebasing; stack_untrack is its opposite, taking a branch and everything above it out of its stack without touching the branches (only: true takes just that branch and restacks the ones above onto its parent); stack_pop deletes the current branch and takes it out of its stack, leaving its changes staged on its parent, and restacks the branches above (its pull request is closed by the next stack_submit). stack_reorder, stack_move and stack_create with insert leave the stack's pull requests to be regrouped on GitHub: the next stack_submit dissolves every stale GitHub stack and creates the ones it did not submit again, its notices naming each (a stack with fewer than two pull requests has nothing to group; a branch without an open pull request, or a failed create, gets a notice saying to submit from that stack); stack_restack rebases the stack locally without checking anything out, bottom branch onto the local trunk included; on a conflict it leaves one git rebase in progress for the files to be resolved; stack_continue finishes an interrupted restack once the conflicts are resolved and git added; stack_abort gives it up and puts every moved branch back; stack_undo takes back the last of those commands run in this worktree (call it again to step further back; list: true shows what it would undo; a successful stack_submit or stack_merge clears it; only pass force: true when the user said to throw away commits made since); stack_navigate moves up/down/top/bottom or checks out a branch; stack_submit pushes the stack (or with down only the current branch and the ones below it) and creates or updates PRs (you write titles and bodies; ready for review unless draft is true); stack_sync fetches, fast forwards trunk, deletes branches whose pull requests merged or closed, fast forwards branches that moved on the remote and restacks every stack without checking anything out; it never pushes. stack_merge merges the stack's pull requests up to a branch into trunk in one all or nothing operation (drafts must be published first) and then syncs; it refuses while any of those pull requests has a check that failed (code checks_failing) or is still running (code checks_pending), listing them in the error's checks field. Fix the failures and push with stack_submit, or wait for running checks and call it again; only pass force: true when the user has said to merge anyway. +const Instructions = `git-stack manages stacked branches and stacked pull requests on top of GitHub's gh stack. Prefer these tools over raw git whenever the user works in a stack: stack_view shows every stack (branches, parents, PR state, needs_restack); stack_create starts a new branch on top of the current one (you write the commit message), or with insert: true right above the current branch in the middle of its stack, restacking the branches above onto it; stack_modify amends the current branch (or adds a commit) and restacks everything above it; stack_absorb folds each staged hunk into the commit of the stack that last changed those lines and rewrites that commit and everything above it (dry_run shows the plan first); use it to fix something lower down without navigating there; stack_reorder changes the order of the current stack's branches (order, bottom to top) and restacks them; stack_move puts a branch and what's above it on top of another stack or on a trunk as a new stack (only: true leaves the branches above behind); stack_fold merges the current branch into the branch below it, one branch and one pull request (keep: true keeps the current branch's name and pull request instead of the parent's), and the next stack_submit closes the pull request of the branch that went; stack_rename gives the current branch a new name, renaming it on GitHub first once it has been pushed so its pull request follows (local: true renames it only in this repository); stack_init tracks branches you already have as a new stack, without rebasing; stack_untrack is its opposite, taking a branch and everything above it out of its stack without touching the branches (only: true takes just that branch and restacks the ones above onto its parent); stack_pop deletes the current branch and takes it out of its stack, leaving its changes staged on its parent, and restacks the branches above (its pull request is closed by the next stack_submit). stack_reorder, stack_move and stack_create with insert leave the stack's pull requests to be regrouped on GitHub: the next stack_submit dissolves every stale GitHub stack and creates the ones it did not submit again, its notices naming each (a stack with fewer than two pull requests has nothing to group; a branch without an open pull request, or a failed create, gets a notice saying to submit from that stack); stack_restack rebases the stack locally without checking anything out, bottom branch onto the local trunk included; on a conflict it leaves one git rebase in progress for the files to be resolved; stack_continue finishes an interrupted restack once the conflicts are resolved and git added; stack_abort gives it up and puts every moved branch back; stack_undo takes back the last of those commands run in this worktree (call it again to step further back; list: true shows what it would undo; a successful stack_submit or stack_merge clears it; only pass force: true when the user said to throw away commits made since); stack_navigate moves up/down/top/bottom or checks out a branch; stack_submit pushes the stack (or with down only the current branch and the ones below it) and creates or updates PRs (you write titles and bodies; ready for review unless draft is true), syncing first when a pull request of the stack has merged since the last sync; stack_sync fetches, fast forwards trunk, deletes branches whose pull requests merged or closed, fast forwards branches that moved on the remote and restacks every stack without checking anything out; it never pushes. stack_merge merges the stack's pull requests up to a branch into trunk in one all or nothing operation (drafts must be published first) and then syncs; it refuses while any of those pull requests has a check that failed (code checks_failing) or is still running (code checks_pending), listing them in the error's checks field. Fix the failures and push with stack_submit, or wait for running checks and call it again; only pass force: true when the user has said to merge anyway. Never run git rebase or git commit --amend by hand inside a stack: descendants would not be restacked. Use stack_modify / stack_restack instead. diff --git a/pkg/mcp/tools.go b/pkg/mcp/tools.go index 3491590..3baa5cc 100644 --- a/pkg/mcp/tools.go +++ b/pkg/mcp/tools.go @@ -538,6 +538,11 @@ func (s *Server) submit(ctx context.Context, req *mcp.CallToolRequest, in submit Publish: in.Publish, Draft: in.Draft, NoEdit: true, DryRun: in.DryRun, UseAI: in.UseAI, Texts: in.PullRequests, Force: in.Force, Down: in.Down, Branch: in.Branch, }) + if err != nil && res.Sync != nil { + // The sync before the submit ran, or failed part way: what it did is + // in the result, so the agent sees what moved. + return &mcp.CallToolResult{IsError: true, Content: []mcp.Content{&mcp.TextContent{Text: wrapErr(err).Error()}}}, res, nil + } if err != nil { return nil, app.SubmitResult{}, wrapErr(err) } @@ -738,7 +743,7 @@ func (s *Server) registerTools() { mcp.AddTool(s.mcp, &mcp.Tool{ Name: "stack_submit", Title: "Submit the stack", - Description: "Push every branch of the current stack and create or update chained pull requests on GitHub; with down (or branch) only the current (or named) branch and the ones below it. New PRs are ready for review unless draft is true (or git config stack.submit.default says draft). Supply pull_requests {branch: {title, body}} for branches without a PR; use dry_run to see the plan first. Refuses with invalid_args when a branch has commits on the remote that aren't local (someone else pushed); pull them in, or pass force only if the user agrees to overwrite them.", + Description: "Push every branch of the current stack and create or update chained pull requests on GitHub; with down (or branch) only the current (or named) branch and the ones below it. New PRs are ready for review unless draft is true (or git config stack.submit.default says draft). Supply pull_requests {branch: {title, body}} for branches without a PR; use dry_run to see the plan first. When a pull request of the stack has merged since the last sync it runs stack_sync first, so the merged branch goes and the ones above are restacked onto trunk before anything is pushed (merged names those branches, sync says what the sync did; a dry run only names them). Refuses with invalid_args when a branch has commits on the remote that aren't local (someone else pushed); pull them in, or pass force only if the user agrees to overwrite them.", Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(true), OpenWorldHint: boolp(true)}, }, s.submit) mcp.AddTool(s.mcp, &mcp.Tool{