github / github/gh-stack

gh stack sync pushed a stack branch's merge commit directly onto the trunk branch (`main`)

Ouverte
#442 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
bug topic: cli - sync
Langage dominant
Go
Étoiles
1.5k
Forks
70
Merge moyen
1 j 8 h
PR mergées (30 j)
7

Description

## Summary

Running `gh stack sync` on an existing 4-branch stack resulted in `origin/main` being advanced to a commit that merges a stack branch's feature commit directly into trunk — outside of any PR merge. The affected PR was never marked merged by GitHub (`mergedAt: null`), yet its code was live on `main`. `sync`'s own output reported full success at every step and gave no indication that a trunk branch had been mutated.

## Environment

- `gh-stack` v0.1.0
- `gh` 2.96.0
- git 2.50.1 (Apple Git-155)
- macOS 15.7.4, arm64

## Stack layout at the time

```
main (trunk)
└─ feature-layer-1 (PR #101, base: main)
└─ feature-layer-2 (PR #102, base: feature-layer-1)
└─ feature-layer-3 (PR #103, base: feature-layer-2)
└─ feature-layer-4 (PR #104, base: feature-layer-3)
```

The stack had already been created with `gh stack init` + `gh stack submit --auto`. Between submit and this incident, `main` had advanced (unrelated PRs merged), and one of the stack branches had also been manually rebased with plain git (outside `gh stack`) to resolve an unrelated conflict. Also the first stacked pr was approved.

## Reproduction

1. With the stack above checked out locally (on the topmost branch), run:
```sh
gh stack sync
```
2. Observed output (abbreviated, all steps reported success):
```
✓ Fetched latest main from origin
✓ Trunk main fast-forwarded to
✓ Fast-forwarded feature-layer-2 to

Rebasing stack ...
✓ Rebased feature-layer-1 onto main
✓ Rebased feature-layer-2 onto feature-layer-1
✓ Rebased feature-layer-3 onto feature-layer-2
✓ Rebased feature-layer-4 onto feature-layer-3

Pushing 4 branches to origin...
✓ Pushed 4 branches

Syncing PRs ...
✓ PR #101 (feature-layer-1) — Open
✓ PR #102 (feature-layer-2) — Open
✓ PR #103 (feature-layer-3) — Open
✓ PR #104 (feature-layer-4) — Open
✓ Stack on GitHub is up to date with 4 PRs (stack #501)

✓ Stack synced
Stacked on main ()
```
3. After this ran, `origin/main`'s tip (verified via `git ls-remote origin refs/heads/main` and the GitHub API `GET /repos/{owner}/{repo}/branches/main`) was a commit that did **not** exist before this command ran.

## Evidence that this was a `sync`-internal push, not a manual one

- `git reflog show main` (local trunk branch) shows only local fast-forwards (`branch: Reset to ...`) — it was **never** locally at the commit that ended up on `origin/main`.
- `git reflog show feature-layer-1` (the bottom-of-stack branch, whose base is trunk) shows:
```
feature-layer-1@{0}: commit (merge): Merge branch 'main' of https://github.com/.../ into feature-layer-1
feature-layer-1@{1}: commit: feat:
feature-layer-1@{2}: branch: Created from origin/main
```
Note the entry type: `commit (merge)`, **not** a rebase. This is despite the CLI's own printed log for this exact step reading `"✓ Rebased feature-layer-1 onto main"`.
- `sha-A` above is byte-identical to `origin/main`'s new tip after `sync` ran (confirmed via `git diff sha-A ` = empty, and matching author/committer/date/message via the GitHub commits API).
- `origin/feature-layer-1` (the actual remote branch for that PR) was, after `sync`, at a **different** SHA than `sha-A` — a proper rebase result, distinct commit. So the merge commit did not simply get pushed to "the wrong copy of the same intended branch"; it ended up on `refs/heads/main` specifically, a ref that `sync` should never write to per its own documented behavior ("Fast-forward trunk" is described as updating the *local* trunk ref to match remote, not pushing to it).
- No `git push` targeting `main` was run manually at any point — every manual `git push` in this session named a specific feature branch explicitly.

## Expected behavior

Per the `gh-stack` README, `sync` step 3 ("Fast-forward trunk") only fast-forwards the **local** trunk ref to match `origin`, and step 4 ("Cascade rebase") rebases stack branches onto their parents. It should never produce a merge commit, and it should never push anything to the trunk ref itself. `sync` should not be able to advance `origin/main` under any circumstances; only a GitHub PR merge should do that.

## Actual behavior

1. For the branch directly based on trunk, the "rebase" step performed a `git merge` of trunk into the branch instead of a rebase (confirmed via reflog entry type), while still reporting it as `"Rebased ... onto main"`.
2. That merge commit was pushed to `origin/main` during the subsequent "Push — pushes all branches" step, even though only the 4 stack branches were named/intended as push targets.
3. `sync` printed full success (`✓ Pushed 4 branches`, `✓ Stack synced`) with no warning, no diff summary, and no indication that a ref outside the 4 stack branches had been written to.
4. This resulted in a stack branch's code landing on the trunk branch of a production repository, completely bypassing that branch's own PR (which GitHub still reported as open/unmerged).

## Impact

This landed code directly on `main` in a production repository, with no error, warning, or confirmation prompt from the tool. For any repo where `main`/trunk is a protected, review-gated branch, this defeats that protection silently.

## Related

- #417 ("gh stack sync reports success after its atomic push fails") documents a related but distinct failure in the same step of `sync` — the atomic push not accurately reflecting/reporting what happened to remote refs. That issue is about a push *failing* while being reported as success; this report is about a push landing somewhere it should never have targeted at all, also reported as success. Both point to the same underlying step ("Push — pushes all branches" in `sync`) having an unreliable relationship between what it does to remote refs and what it tells the user.

## Suggested fix direction

- `sync`'s push step should construct its list of push refspecs explicitly from the known stack branch names only, and should hard-fail (never silently succeed) if a trunk/base ref is ever included in that list or if any pushed ref does not exactly match one of the stack's tracked branch names.
- The "Fast-forward trunk" and "Cascade rebase" steps should be implemented with real `git rebase`, not `git merge`, for the bottom-of-stack branch — and the reflog/commit shape should be asserted in tests to catch exactly this kind of mislabeling.
- Consider printing the actual SHA delta being pushed per branch (old → new) in `sync`'s output, so a trunk-ref write would be visible immediately rather than silent.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Commencez par l’étape de push de `gh stack sync` et ses points d’entrée `Fast-forward trunk` et `Cascade rebase`, puis reproduisez le scénario documenté d’une stack à quatre branches. C’est terminé lorsque seuls les branches de la stack suivis sont poussés, que la branche du bas utilise un vrai rebase sans créer de merge commit, et que toute différence dans la refspec de trunk ou dans le push échoue visiblement au lieu de signaler un succès.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
git, github, go
Domaine
cli, devtools
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Clairement spécifiée
Accessibilité débutants
48/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.