Skip to content

Add 'downstack merge' command to merge stacks#1152

Merged
abhinav merged 21 commits into
mainfrom
branch-merge
May 31, 2026
Merged

Add 'downstack merge' command to merge stacks#1152
abhinav merged 21 commits into
mainfrom
branch-merge

Conversation

@ed-irl

@ed-irl ed-irl commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

This adds gs downstack merge as an experimental command
for merging a stack of Change Requests bottom-up through the forge API.

The command acts as a local merge queue.
It merges the lowest open Change Request first,
waits for that merge to settle,
syncs trunk,
restacks and updates the next branch,
waits for CI checks on the updated Change Request,
and repeats until the requested downstack has been merged.

The command is gated behind the downstackMerge experiment
while the merge-queue behavior is still settling.

Merge flow

For a stack like:

main <- feature1 <- feature2 <- feature3

running gs downstack merge from feature3 merges:

feature1, feature2, feature3

After each merge,
repo sync updates trunk,
deletes merged branches,
and retargets their upstacks.
The next queued branch is then restacked,
submitted as an update to its existing Change Request,
checked for passing CI,
and merged.

This keeps squash-merge handling in the merge queue itself,
rather than relying on delete cleanup to repair the stack afterward.

Test support

ShamHub now supports:

  • shamhub config mergeMethod merge
  • shamhub config mergeMethod squash

This lets script tests exercise server-side squash merge behavior
for gs downstack merge.

Refs #65

@ed-irl

ed-irl commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author

@ed-irl ed-irl requested a review from abhinav May 12, 2026 12:55
@ed-irl ed-irl force-pushed the branch-onto-resolve-from-ref branch from 265d671 to 3e389f6 Compare May 12, 2026 13:49
@ed-irl ed-irl changed the title feat(branch): add merge command for downstack PRs Add gs branch merge (bm) and gs ci merge-guard for stacked PR merging May 12, 2026
@ed-irl ed-irl changed the title Add gs branch merge (bm) and gs ci merge-guard for stacked PR merging Add gs branch merge (bm) for stacked PR merging May 12, 2026
@ed-irl ed-irl force-pushed the branch-onto-resolve-from-ref branch from 3e389f6 to 0d9161c Compare May 12, 2026 15:11
@abhinav abhinav force-pushed the branch-onto-resolve-from-ref branch from 0d9161c to 19e5015 Compare May 17, 2026 21:51
@abhinav abhinav changed the base branch from branch-onto-resolve-from-ref to sync-survivingancestor-loop May 23, 2026 16:22
@abhinav abhinav force-pushed the sync-survivingancestor-loop branch from e4b5f2e to ddbdacb Compare May 23, 2026 18:50
Base automatically changed from sync-survivingancestor-loop to main May 23, 2026 18:58
ed-irl and others added 2 commits May 23, 2026 12:41
Introduce `gs branch merge` (gs bm) to merge pull requests via the
forge API and clean up merged branches locally.

Key behaviors and safeguards:

- Fetch trunk after each merge completes so upstack rebases use
  the latest remote state, avoiding silent change loss.
- Halt with an error if any branch has unpushed local commits,
  preventing silent data loss during post-merge cleanup.
- Assert the expected head SHA at merge time via forge APIs
  (GitHub ExpectedHeadOid, GitLab SHA, ShamHub) to reject races
  where the remote branch changes between plan and merge.
- After moving an immediate child onto the new base, restack all
  further descendants in topological order so the entire upstack
  chain stays consistent without manual intervention.
The branch merge command still expected stale-base validation
to live behind its older command-local API, but PR #1154 moved
that behavior into submit-specific preflight code.

Extract stale-base detection into `spice.FindStaleBases` so submit
and branch merge use the same graph and forge-state check. Branch
merge now mirrors submit diagnostics while keeping its own
`--no-branch-check` bypass wording.

Refs #1154
abhinav added 2 commits May 23, 2026 15:03
Submit commands should not require a supported forge just to discover
that no stale-base check is needed.

`FindStaleBases` now accepts a repository opener and calls it only
after local branch graph analysis finds candidate base changes.
This keeps `--no-publish` usable with unsupported remotes while
preserving stale-base validation when published downstack changes exist.
- Move the merge command from `branch merge` to `downstack merge`
  so the public command reflects that it operates on a downstack.
  Keep room for a future single-branch `branch merge` command.
- Expand command help with a stack diagram, merge order,
  blocking behavior, and the local merge queue model.
- Resolve the default branch in `AfterApply`, matching nearby command
  patterns and removing the separate branch resolution helper.
- Move stale-base validation into the merge handler so command wiring
  stays thin, matching the submit handler boundary.
- Rename `ChangeChecksStatus` to `ChangeChecksState` so the method
  and return type use symmetrical state terminology.
- Add documented forge status constants for GitHub, GitLab,
  and Bitbucket, including links to the upstream API references.
  GitHub `EXPECTED` now remains pending instead of passing.
- Inline one-use merge/check helpers and add error context around
  handler and repository-provider failures.
- Remove the delete-handler descendant restack behavior because branch
  deletion should not automatically restack the full descendant chain.
- Make the merge confirmation default to proceed in non-interactive mode,
  following the view behavior expected by other commands.
- Replace timer allocation with `time.After` now that timers are
  collected automatically.

Out of scope:

- Add a future single-branch `branch merge` command.
- Report individual blocking status names in merge errors.
- Teach merge waiting about server-side merge queues and richer merge
  actions.
- Replace merge cleanup with `sync.Handler` after issue #1134 is fixed.
@abhinav abhinav marked this pull request as draft May 24, 2026 21:27
@abhinav

abhinav commented May 24, 2026

Copy link
Copy Markdown
Owner

@ed-irl I'm still working through this. Marked it as draft so I don't accidentally merge it.
I'm hoping to have this in this weekend.
One note:
I'm renaming this to downstack merge since that's the scope. That'll allow a "merge only this branch" branch merge command in the future as well as a stack merge that somehow handles upstack fan out.

Update: I've made a bunch of changes.
This became a forcing function for finishing up #1197, which bubbled into #1199, #1201.

abhinav added 3 commits May 24, 2026 17:11
ShamHub needs to simulate forge defaults that are not exposed
through the git-spice merge command yet.
Add a test-script configuration command so scripts can choose
how REST merge calls behave when they omit a merge method.

The new `shamhub config mergeMethod <value>` surface accepts
`merge` and `squash`.
The REST merge endpoint uses that setting as its default,
while direct merge requests can still specify their method explicitly.
abhinav added 3 commits May 24, 2026 18:27
`downstack merge` needs to replay each surviving branch after its base
is merged on the forge.
The first review pass removed that behavior from branch deletion,
which was the right boundary,
but squash merges still need the branch replay before the next Change
Request can be updated and merged.

Move that work into the merge queue itself.
After a merge settles,
the handler syncs trunk through the repo sync handler,
restacks the next branch,
updates its Change Request through the submit handler,
waits for checks on the updated head,
and then continues the queue.

Add a ShamHub squash-merge regression test that merges multiple
branches through `downstack merge`.
The test covers the case where the server creates squash commits,
so the next branch must be replayed before it can be merged safely.
`--no-wait` is a fire-and-forget mode for a single merge
request.
It cannot safely drive the local merge queue because each later
branch needs the previous merge to settle before sync, restack,
submit, and checks can run.

Reject multi-branch `downstack merge --no-wait` plans before
prompting or merging.
For the waited path, let repo sync own merged branch cleanup and
merged-downstack bookkeeping instead of deleting branches directly
from the merge handler.

The no-wait script covers both the multi-branch rejection and the
single-branch fire-and-forget path.
abhinav added a commit that referenced this pull request May 30, 2026
Move stale-base detection into `spice.FindStaleBases`
so submit validation can use a shared graph and forge-state check
instead of keeping that logic inside the submit handler.

The submit handler still owns its user-facing diagnostics.
It logs each stale local branch/base edge
and returns the existing aggregate error
that asks the user to run `gs repo sync`
or submit with `--force`.

`FindStaleBases` accepts a repository opener
and calls it only after local graph analysis finds candidate bases.
This keeps submit flows that do not need stale-base validation
from requiring a supported forge.

Extracted from #1152
abhinav added a commit that referenced this pull request May 31, 2026
Extracted from #1152

This adds forge-level APIs for merging changes
and reading aggregate CI/checks state.
GitHub, GitLab, Bitbucket, and ShamHub now expose the new forge methods
where their backing APIs support them.

Checks-state integration tests now cover pending, passing,
and failing checks for ShamHub, GitHub, and GitLab.

ShamHub also gained support for simulating checks state
through its existing test forge system,
including a `shamhub set-status` command
for setting a PR's synthetic checks state during tests.

Finally, this removes the unused `ErrMergeNotAllowed` sentinel
from the forge contract.
It was only declared and documented here,
and was unused in the `branch-merge` branch as well.

---------

Co-authored-by: Edmund Kohlwey <ed@irl.llc>
#: Conflicts:
#:	internal/forge/forge.go
#:	internal/forge/shamhub/checks.go
#:	internal/forge/shamhub/cli.go
#:	internal/gateway/bitbucket/api.go
#:	internal/gateway/gitlab/api.go
@abhinav abhinav changed the title Add gs branch merge (bm) for stacked PR merging Add 'downstack merge' command to merge stacks May 31, 2026
abhinav added 6 commits May 31, 2026 06:58
Downstack merge now has script coverage for the CI states
that must stop the local merge queue before or after a
server-side merge.

The tests assert full stderr for failed and pending checks,
and use `cmpenvJSON` fixtures for ShamHub change state.
That keeps the open and merged CR contracts visible without
baking in test server ports.

ShamHub API merges now get a deterministic default commit time
from the CR head when no explicit time is provided.
That keeps server-side merge SHAs stable for script fixtures
while preserving the existing CLI merge path that can supply
a script time.

The test-script skill documents the `cmp` to `--update` to
`$SHAMHUB_URL` workflow for ShamHub JSON dumps.
Sync retargets surviving upstack CRs on the forge after merged
branch cleanup retargets the local stack metadata.
Both operations are useful, but the old forge progress line looked
like a duplicate of the local retarget message.

Report forge retarget progress with the branch name and Change ID,
such as `feature2: retargeting #2 onto main`, so the output makes
clear that the CR is being edited.

The merge queue no longer emits its own pre-restack progress line.
It relies on the restack handler's completion log instead, matching
repo sync's ownership of restack output.

Script coverage was swept for the changed sync retarget log shape,
and the direct merge retarget helper keeps its existing wording for
paths that do not go through sync.
When `downstack merge` cannot compare a branch with its remote
tracking ref, it still avoids blocking the merge as a false positive.
That path is expected to be unusual because submitted branches normally
have remote-tracking refs.

Emit a warning with the branch name, remote ref, and underlying error
before skipping the check. This keeps the current non-blocking behavior
while making the missing safety signal visible to the user.
Mark `gs downstack merge` experimental while the local merge queue
behavior is still settling.

The merge plan now uses one branch graph snapshot to derive the
downstack queue, change metadata, and stale-base validation. During
execution, each queued change is checked against trunk, restacked only
when needed, submitted as an update-only existing CR, and logged with
the change URL before merging.

This keeps squash-merge handling in the merge queue instead of delete
cleanup, so the next branch can be restacked, submitted, checked, and
merged after its base lands.

Script coverage exercises the experimental gate, squash merges, CI
failures on the next queued branch, and the updated progress output.
@abhinav abhinav marked this pull request as ready for review May 31, 2026 19:54
@abhinav abhinav merged commit a64e5dd into main May 31, 2026
26 of 46 checks passed
@abhinav abhinav deleted the branch-merge branch May 31, 2026 20:16
@abhinav

abhinav commented May 31, 2026

Copy link
Copy Markdown
Owner

Thanks for the change, @ed-irl!
Minus the rename, I simplified some of the logic to take advantage of improvements to state handling in sync handler, delete handler, etc.
I've also marked this feature experimental/opt-in because I expect to make some changes to it in the near future.

@abhinav abhinav mentioned this pull request Jun 2, 2026
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants