Skip to content

fix for rev-parse error during sync with deleted branches#42

Merged
skarim merged 2 commits into
skarim/fix-rebase-deletedfrom
skarim/fix-sync-deleted
Apr 20, 2026
Merged

fix for rev-parse error during sync with deleted branches#42
skarim merged 2 commits into
skarim/fix-rebase-deletedfrom
skarim/fix-sync-deleted

Conversation

@skarim

@skarim skarim commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

Fix rev-parse error during sync with deleted branches

Same root cause as the rebase fix, but in the gh stack sync code path. The branch-ref snapshot before rebase included deleted branches, causing a rev-parse failure.

Fix: Apply the same guard — skip merged branches that don't exist locally when building the ref snapshot.

Copilot AI review requested due to automatic review settings April 16, 2026 14:58
@skarim skarim changed the title skarim/fix sync deleted fix for rev-parse error during sync with deleted branches Apr 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates gh-stack’s sync/rebase behavior and documentation around handling merged PRs (previously described as “squash-merged”), including a new test case for merged branches that no longer exist locally (e.g., deleted after merge).

Changes:

  • Adjusts sync’s pre-rebase “original refs” capture to skip merged branches that don’t exist locally.
  • Renames “squash-merged” terminology to “merged” across CLI docs, README, skill docs, messages, and tests.
  • Adds/updates tests to cover merged-branch --onto behavior and a “merged branch deleted from remote” scenario.
Show a summary per file
File Description
skills/gh-stack/SKILL.md Updates user-facing documentation wording from squash-merge to merged PR handling.
internal/git/git.go Updates RebaseOnto doc comment terminology to “merged”.
docs/src/content/docs/reference/cli.md Updates CLI reference text to describe merged PR detection for --onto.
cmd/sync_test.go Renames tests and adds coverage for merged-branch scenarios, including missing local branch.
cmd/sync.go Skips non-existent merged branches when collecting SHAs for conflict-restore bookkeeping.
cmd/rebase_test.go Renames tests/comments to match “merged” terminology.
cmd/rebase.go Updates comments/messages to match “merged” terminology.
README.md Updates README description of merged PR handling during rebase.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

cmd/rebase.go:209

  • The merged-branch skip path sets ontoOldBase = originalRefs[br.Branch]. If a merged branch was deleted locally and therefore excluded from the RevParseMap inputs, this lookup will be empty and subsequent RebaseOnto(newBase, ontoOldBase, ...) will run with an invalid oldBase. Consider using the persisted SHA from stack metadata (e.g., br.Head) when the ref is missing, and fail fast with a clear message if you can't determine an oldBase SHA.
		// Skip branches whose PRs have already been merged.
		// Record state so subsequent branches can use --onto rebase.
		if br.IsMerged() {
			ontoOldBase = originalRefs[br.Branch]
			needsOnto = true
			cfg.Successf("Skipping %s (PR %s merged)", br.Branch, cfg.PRLink(br.PullRequest.Number, br.PullRequest.URL))
  • Files reviewed: 8/8 changed files
  • Comments generated: 3

Comment thread cmd/sync.go
Comment thread cmd/sync.go
Comment thread cmd/sync_test.go
@skarim skarim force-pushed the skarim/fix-rebase-deleted branch from 08a7f7b to fba004c Compare April 17, 2026 05:28
@skarim skarim force-pushed the skarim/fix-sync-deleted branch from d277c87 to 63f019e Compare April 17, 2026 05:28
@skarim skarim merged commit eac53c2 into main Apr 20, 2026
6 checks passed
@skarim skarim deleted the skarim/fix-sync-deleted branch April 20, 2026 14:17
skarim added a commit that referenced this pull request Apr 20, 2026
* fix for rev-parse error during sync

* clearer info msg with rebasing over merged PRs
ryanclark added a commit to ryanclark/gh-stack that referenced this pull request Jun 19, 2026
Ports the still-applicable upstream changes since the fork point, adapted
to this fork's standard-GitHub-API architecture (no private stack backend;
stack deps tracked via "Requires #N"). Each change was test-driven.

Bug fixes:
- github#56  guard GraphQL PR-number conversion against int32 overflow
- github#41  skip merged+deleted branches when rebasing (rev-parse error)
- github#42  same fix on the sync path
- github#43  --onto rebase for merged branches: backfill refs, pre-seed onto
       state, stale-base fallback via merge-base
- github#39  always anchor branch diffs on the merge-base (inflated diff counts)
- github#95  sync now cascades when the stack is stale even if trunk is current;
       rebase skips queued branches (IsSkipped)
- github#80  view --json resolves directly with typed exit codes (no prompt)
- github#49  (Part A) ignore stale merged/closed PRs for reused branch names
       (the backend ListStacks path is intentionally omitted)

Features (standard API only):
- github#108 trunk command       - github#51  switch command
- github#74  unstack -> active    - github#101 add adopts existing branch
- github#40  fast-forward branches behind their remote
- github#76  draft PRs by default + --open (adds MarkPRReadyForReview)
- github#77  seed PR body from repo template (keeps "Requires #N")
- github#94  sync --prune merged branches

Infra / cleanup:
- github#103/github#110 bump github.com/cli/cli/v2 to v2.93.0 (go get + tidy)
- github#89  remove placeholder `merge` command + docs
- fix stale `unstack [branch]` docs

Note: this commit is layered on in-progress local work (go-git ops,
PR/commit caching, discover command) that shares the same files, so the
two are committed together. Full suite green: build, vet, go test ./...
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.

3 participants