mirror of
https://github.com/github/gh-stack.git
synced 2026-09-14 20:26:28 +08:00
v0.0.8
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Sameen Karim
|
0c51c08255 |
Fix to properly revert after aborting modify (#167)
* Add cherry-pick abort/quit/in-progress git primitives Introduce IsCherryPickInProgress() (detects .git/CHERRY_PICK_HEAD) and split the existing cherry-pick reset into two distinct operations: - CherryPickQuit() runs `git cherry-pick --quit`, clearing the sequencer state without touching the index (used to clear stale state before starting a fresh cherry-pick). - CherryPickAbort() now runs `git cherry-pick --abort`, which fully restores the working tree and index to the pre-cherry-pick state. The previous CherryPickAbort() ran --quit, which leaves an unmerged index and therefore cannot recover a conflicted fold-down. Integration tests cover the in-progress detection, the full abort restore, and the --quit-leaves-index behavior. * Fix modify --abort leaving a broken stack after a conflict When `gh stack modify` hit a rebase or cherry-pick conflict it saved state with phase "conflict" and told the user to run `gh stack modify --abort` to restore. But runModifyAbort had no case for PhaseConflict, so it fell into the default branch that merely printed "unexpected modify state phase" and deleted the state file without unwinding. The in-flight rebase/cherry-pick stayed active, branches were left partially rewritten, and the deleted state file also made --continue impossible: the stack was stuck in limbo. Fixes: - runModifyAbort now unwinds on PhaseConflict (same recovery as PhaseApplying), aborting the in-progress operation, resetting branch tips to their pre-modify SHAs, restoring stack metadata, and clearing state. - Unwind now also aborts an in-progress cherry-pick (fold-down conflicts), not just a rebase. Without this the restore checkouts would fail on the unmerged cherry-pick index. - ContinueApply now records a subsequent cascade-rebase conflict as ConflictType "rebase" instead of leaving a stale "cherry_pick", so the next --continue calls RebaseContinue rather than failing in CherryPickContinue. Adds coverage for the conflict-phase abort, pending-submit no-op abort, Unwind aborting an active cherry-pick, and the cherry-pick to rebase ConflictType transition. * Persist fold-branch removal when a post-fold cascade rebase conflicts ContinueApply removes the folded branch from the in-memory stack after a fold-down cherry-pick is resolved, but a subsequent cascade rebase conflict only saved the modify state file, not the stack metadata. On the next --continue the on-disk metadata (folded branch still present) was re-read, and because ConflictType is now "rebase" the fold-removal block was skipped, so the final save resurrected the folded branch as a phantom entry pointing at an orphaned tip. Persist the stack file alongside the state file on a cascade-rebase conflict, mirroring ApplyPlan's save-on-conflict, so the fold removal survives recovery. Adds an end-to-end regression test covering the fold-then-cascade-conflict path across two --continue calls. |
||
|
Sameen Karim
|
754d190490 |
Fork unmerged branches into new stack (#154)
* Fork unmerged branches into a new stack when the base stack is fully merged
Once every PR that is officially part of a stack on GitHub has been merged
-- especially after the merged branches are deleted upstream -- you can no
longer add to that stack. A new PR on top would target the trunk directly
instead of chaining onto the merged PRs, so the remote stack's "each PR's
base ref is the previous PR's head ref" invariant no longer holds. On the
next `gh stack submit`, the stack update was rejected and surfaced as a
confusing, dead-end warning:
Failed to update stack on GitHub: Pull requests must form a stack,
where each PR's base ref is the previous PR's head ref
`submit` had no handling for this: `syncStack` always sent the full PR list
(including the merged-and-deleted ones), so the API rejected the broken
chain even though the new PRs had already been created with correct bases.
Fork the survivors into a fresh stack instead of failing. After syncing PR
state and before pushing, `runSubmit` now calls `maybeForkFromMergedBase`:
- It triggers only when every PR officially part of the tracked remote
stack (`s.ID`) has merged. Membership is read from the stacks API, so
open PRs that are not part of the remote stack do not count, and -- this
is the key guard -- a normal partial, bottom-up merge (where the remote
stack still lists an open PR) is left completely untouched. A cheap
pre-check (the local stack must have at least one merged branch) avoids
an extra ListStacks call on the common path.
- The local branches are partitioned: those still in the merged remote
stack stay behind; everything else (new branches, plus open PRs that were
never part of that remote stack) is lifted into a brand-new stack rooted
at the original trunk, with an empty remote ID. The bottom survivor is
re-based onto the trunk.
- `runSubmit` continues with the new stack, so the push loop, PR creation,
and `syncStack` all operate on it; the empty ID routes `syncStack` through
the adopt/create path and a fresh stack is created on GitHub.
- The original, fully merged stack is left untouched on GitHub. Locally it
is kept as a record only if at least one of its branches still exists in
the working copy; otherwise it is dropped. No data is lost -- those PRs
are already merged on GitHub.
To restructure the stack file safely, add `StackFile.IndexOfStack`, which
locates a stack by pointer identity so the fork can capture what it needs
before `AddStack`/`RemoveStack` reallocate the underlying slice.
Also soften the partial-merge case that does not fork: when an `UpdateStack`
call fails with the "must form a stack" 422 and the stack still contains
merged branches, report it as an informational note (the unmerged PRs were
pushed and re-based onto the trunk) rather than a scary failure warning.
Scope is limited to `submit`. `add` and `checkout` keep their existing
"refuse and suggest `gh stack init`" behavior on fully merged stacks.
Tests:
- cmd/submit_test.go: TestSubmit_ForksWhenRemoteStackFullyMerged covers both
disposition variants (the old stack is removed when its merged branches
are gone locally, kept when they still exist) and asserts that only the
new branches are pushed, the fork message is printed, a fresh stack is
created, and the local stack file is split into two stacks.
TestSubmit_NoForkWhenRemoteStackHasOpenPR verifies the everyday bottom-up
merge is not forked and that the broken-chain 422 is reported calmly.
TestUpdateStack_BrokenChainAfterMerge checks the calm-vs-warn branch.
- internal/stack/stack_test.go: TestIndexOfStack covers identity lookup and
the not-found case.
Docs: README, the CLI reference, the stacked-PRs guide, the FAQ, and the
agent SKILL.md note that submitting onto a fully merged stack starts a new
stack rooted at the trunk.
* Handle fully merged stacks gracefully in the view and modify TUIs
Merged branches (and their PRs) are not selectable, so once an entire stack
has landed there is nothing to act on -- yet the TUIs did not reflect that:
- `gh stack view` still drew a highlighted cursor on the top branch even
though it could not be selected. Navigation, checkout, and the per-branch
toggles all silently did nothing, with no indication of why.
- `gh stack modify` opened its full editor on a stack with nothing left to
restructure, instead of short-circuiting like `gh stack submit` does when
there is nothing to submit.
Reflect the "nothing actionable" state in both TUIs.
View (internal/tui/stackview/model.go):
- Hide the cursor when every branch is merged. `New` now starts the cursor
at -1 and only lands it on the current or first non-merged branch; when
none exists the cursor stays hidden, so no row is rendered as focused. The
existing `m.cursor >= 0` guards and merged-skipping `moveCursor` already
make every cursor action a no-op in that state, and mouse-wheel scrolling
still works for tall merged stacks.
- Dim the shortcuts that depend on the cursor. `buildHeaderConfig` marks
navigate, commits, files, open PR, and checkout as `Disabled` (rendered
gray via the existing ShortcutEntry.Disabled styling) when all branches
are merged, leaving only `q quit` active.
Modify (cmd/modify.go):
- Short-circuit before opening the TUI. After preconditions pass and PR
state is synced, `runModify` now returns early when the stack is fully
merged, printing "All branches in this stack have been merged" and
pointing at `gh stack init`, exiting cleanly (exit 0) like submit's
"nothing to submit" path. The linearity and merge-queue precondition
checks already skip merged branches, so they do not fire spuriously.
Tests:
- internal/tui/stackview/model_test.go: the cursor is hidden (-1) when all
branches are merged; up/down/enter do not move it or trigger a checkout;
View renders without panicking on a hidden cursor; buildHeaderConfig
disables every cursor-dependent shortcut (and only those) when all merged,
and leaves them all enabled when active branches remain.
- cmd/modify_test.go: runModify short-circuits on a fully merged stack,
printing the message and returning no error without launching the TUI.
|
||
|
Sameen Karim
|
756002b4c9 |
ensure local trunk branch for required operations (#127)
* Ensure trunk branch exists locally before commands that need it When a user starts a stack after renaming their initial branch (e.g. `git branch -m newbranch`), the trunk branch (e.g. main) may not exist as a local branch. Commands that pass the trunk name to git operations like merge-base, rebase, or rev-parse then fail with: fatal: Not a valid object name main Add an `ensureLocalTrunk` helper that checks whether the trunk branch exists locally and, if not, fetches it from the remote and creates a local tracking branch. This mirrors the pattern already used in the checkout command for importing stacks. Commands updated: - modify: call ensureLocalTrunk before the linearity check in CheckStackLinearity, which uses IsAncestor(trunk, branch). This was the originally reported failure. - rebase: call ensureLocalTrunk after fetch and before fastForwardTrunk and the cascade rebase. git rebase requires a locally resolvable ref; the remote tracking ref alone is not sufficient. - trunk: call ensureLocalTrunk before CheckoutBranch so that `gh stack trunk` works even when trunk was never created locally. - checkout: refactor the existing inline BranchExists + CreateBranch block to use the shared helper. Also fix an incorrect comment in fastForwardTrunk that claimed "the remote tracking ref is sufficient for rebasing" — verified empirically that `git rebase main` fails when main has no local branch, even after fetching origin/main. Commands that were already safe and required no changes: - sync: fetches trunk explicitly and fastForwardTrunk guards with BranchExists - push, switch, navigate, unstack: do not reference trunk - add, submit: do not require trunk as a local git ref - view: handles IsAncestor errors gracefully (false positive is acceptable since rebase will fix it) * add check to avoid unnecessary remote selection prompt |
||
|
Sameen Karim
|
49a753708c |
insert branches with modify (#107)
* add insert branch operation to modify TUI
Add `i` (insert below) and `I` (insert above) key bindings to the
interactive modify view, allowing users to insert new empty branches
into an existing stack. This follows Vim-inspired semantics where
lowercase `i` inserts below the cursor and uppercase `I` inserts above.
## TUI behavior
When the user presses `i` or `I`, the TUI enters an insert input mode
(similar to rename mode) where they type a new branch name. The input
is validated against git ref naming rules, local branch uniqueness, and
in-stack name collisions. On confirm, a placeholder node is inserted at
the correct position in the branch list with a green "✚ insert"
annotation badge and green connector styling.
Insert is a structure operation — it works alongside fold, rename, and
drop, but is mutually exclusive with reorder (consistent with existing
mode exclusivity rules). Undo (`z`) removes the inserted node cleanly.
## Apply engine
At apply time (Step 2 in the pipeline, between renames and folds), the
engine creates the new git branch at the parent branch's tip via
`git.CreateBranch` and inserts a `BranchRef` into the stack metadata at
the correct position. If the insertion changes the base of a branch
that has an open PR, `affectsPRs` is set to trigger a required
`gh stack submit` afterward.
## Header shortcut updates
- Combined the fold shortcuts into a single line: `d/u - fold down/up`
- Added insert shortcuts on their own line: `i/I - insert below/above`
- Reordered fold references throughout to list "down" before "up" for
consistency with the insert shortcut ordering
## Files changed
- types.go: ActionInsertBelow/ActionInsertAbove types, IsInserted field,
InsertedBranches in ApplyResult
- model.go: key bindings, insert input mode, undo, mode exclusivity,
annotation, styling, header shortcuts, effective-index tracking to
prevent false reorder detection when inserts shift node positions
- styles.go: green insert badge/branch/connector styles
- status.go: insert counting in pending change summary
- help.go: new "Insert below / above" section, reordered fold heading
- apply.go: BuildPlan and ApplyPlan handle insert actions
- modify.go: updated command description and success summary
- README.md: updated keybindings table
## Test coverage
- 16 new TUI tests: insert below/above, top/bottom edges, undo, mode
exclusivity, merged branch guard, cancel/empty input, duplicate name
validation, pending summary counting, annotation rendering, mixed
operations with drop/fold, apply acceptance
- 4 new apply tests: BuildPlan produces correct insert actions,
ApplyPlan creates branches and updates stack metadata, insert at
stack start uses trunk as parent, affectsPRs triggered when inserting
before a branch with an open PR
* update add error msg to direct users to modify for inserting branches
* docs updates
* fix insert branch bugs in modify TUI
Fix three bugs with the insert branch feature in the modify TUI, and
adjust rename behavior on inserted nodes.
## Bug 1: False "moved" annotations on existing branches
After inserting a branch, all branches below the insertion point
displayed "↕ moved 1 layer down" annotations. This happened because
`nodeAnnotation` and `toNodeData` compared each node's
`OriginalPosition` against its raw array index, which gets shifted
when an inserted node is added to the slice.
Fix: introduce an `effectiveIdx` parameter that counts only
non-inserted nodes, so position comparisons reflect the original
ordering. The View loop computes effective indices by incrementing
only for non-inserted nodes and passes them to the rendering
functions.
## Bug 2: Header branch count inflated by staged inserts
The branch count in the header ("N branches") included inserted
placeholder nodes, making it appear as though the stack had grown
before changes were applied.
Fix: `buildHeaderConfig` now excludes `IsInserted` nodes from the
branch count. The count reflects only the original branches in the
stack.
## Bug 3: Operations allowed on inserted placeholder nodes
Inserted nodes could be folded into other branches, which makes no
sense for a placeholder with no commits. Additionally, the "last
branch" guard counted inserted nodes as active, allowing users to
drop all original branches and bypass the empty-stack check.
Fix:
- `fold()` rejects inserted nodes with a descriptive error message.
- `toggleDrop()` on an inserted node removes it entirely and pops
the original insert action from the undo stack (clean cancellation
rather than a separate undo entry).
- All three "active branch" guards (`toggleDrop`, `fold`, `tryApply`)
now exclude `IsInserted` nodes, ensuring at least one original
branch always remains in the stack.
## Rename on inserted branches
Instead of blocking renames on inserted nodes, pressing `r` now
enters rename mode and updates the insert action's name in place.
The node's `Ref.Branch` and `PendingAction.NewName` are both updated
directly — no separate rename action is created in the undo stack.
This lets users fix a typo without having to drop and re-insert.
## Tests added
- `TestInsertDoesNotShowMovedAnnotation` — verifies no false move
annotations appear on existing branches after an insert
- `TestBranchCountExcludesInserts` — verifies header count stays
stable after insert
- `TestCannotFoldInsertedBranch` — verifies fold is blocked
- `TestCannotRenameInsertedBranch` — verifies rename updates the
insert name in place
- `TestDropInsertedBranchRemovesIt` — verifies drop removes the node
- `TestDropInsertedBranchCanBeUndone` — verifies drop pops the
original insert from the undo stack
- `TestCannotDropAllOriginalBranchesWithInsert` — verifies the
empty-stack guard excludes inserted nodes
* ensure cannot fold into an inserted branch
* rm dead code
* delete inserted branches during abort
|
||
|
Sameen Karim
|
9cc827dc78 |
modify: only require submit when changes affect PRs (#106)
* modify: only require submit when changes affect PRs Previously, `gh stack modify` always transitioned to `PhasePendingSubmit` after completing on any stack with a remote ID (`s.ID != ""`). This blocked the user from running another `modify` until they ran `gh stack submit`, even when the modifications only touched local branches without PRs. This was overly restrictive. If a user is working at the top of their stack with branches that haven't been pushed or had PRs created yet, restructuring those branches is a purely local operation — there is no remote state to reconcile, and no reason to force a submit before allowing further modifies. ## What changed The condition for entering `PhasePendingSubmit` is now `s.ID != "" && affectsPRs` instead of just `s.ID != ""`. A new `affectsPRs` flag is tracked throughout the apply process. It is set to `true` when any of the following occurs: - A **renamed** branch has a `PullRequest` ref - A **folded** branch (source or target) has a `PullRequest` ref - A **dropped** branch has a `PullRequest` ref - A **rebased** branch (during cascading rebase) has a `PullRequest` ref If none of these conditions are met, the modify state file is cleared immediately — no pending-submit lock, no "run `gh stack submit`" prompt. ## Changes by file **`internal/modify/state.go`** - Added `AffectsPRs bool` field to `StateFile`. This persists the flag across conflict boundaries so that `ContinueApply` knows whether actions applied before the conflict already affected PR branches. **`internal/modify/apply.go`** - `ApplyPlan`: tracks `affectsPRs` through each step (rename, fold, drop, rebase). Saves the flag into conflict state when a conflict occurs. Uses `s.ID != "" && affectsPRs` for the pending-submit decision. - `ContinueApply`: initializes `affectsPRs` from the saved state file, then checks the conflict branch and remaining branches for PRs during the cascading rebase. Uses the same combined condition. - Both functions set `result.NeedsSubmit` / show the "run submit" message only when the flag is true. **`internal/tui/modifyview/types.go`** - Added `NeedsSubmit bool` to `ApplyResult` so the caller can use it for the success message. **`cmd/modify.go`** - `printModifySuccess` now takes its cue from `result.NeedsSubmit` instead of `s.ID != ""`. The "run `gh stack submit`" hint is only shown when PR branches were actually affected. **`internal/modify/apply_test.go`** - Updated `TestApplyPlan_PendingSubmitForRemoteStack` to use branches with PRs and trigger an actual rebase, validating the pending-submit path correctly. - Added `TestApplyPlan_ClearsStateForRemoteStackWithNoPRBranches`: remote stack where no branches have PRs → state is cleared. - Added `TestApplyPlan_PendingSubmitOnlyWhenPRBranchesAffected`: remote stack with a mix of PR and non-PR branches, only the non-PR branch is renamed → state is cleared, `NeedsSubmit` is false. ## Behavior summary | Scenario | Before | After | |---|---|---| | Modify on local stack (no remote ID) | State cleared | State cleared (unchanged) | | Modify on remote stack, PR branches affected | `PhasePendingSubmit` | `PhasePendingSubmit` (unchanged) | | Modify on remote stack, only local branches affected | `PhasePendingSubmit` ❌ | State cleared ✅ | The `CheckStateGuard` function (used by `add`, `push`, `sync`, `unstack`, `rebase`) already did not block on `PhasePendingSubmit`, so those commands are unaffected by this change. * clear state after saving stack * clarify submit requirement in description * assign value directly |
||
|
Sameen Karim
|
8a9f1c86f8 |
commands help text (#88)
* more help text for commands * improved root help * fix typo Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * updated examples for add cmd Co-authored-by: Sameen Karim <skarim@github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Sameen Karim
|
8cddfd4e21 |
Optimize view/modify TUI load time with parallel fetching (#79)
- Deduplicate API calls: syncStackPRs now returns PRDetails for LoadBranchNodes to reuse, eliminating redundant FindPRDetailsForBranch calls - Parallelize API calls in syncStackPRs (capped at 6 concurrent requests) - Parallelize git operations in LoadBranchNodes (capped at 4 concurrent) - Show "Loading stack..." indicator for interactive sessions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Sameen Karim
|
7a268fc380 |
modify command (#72)
* git primitives for modify cmd * extract reusable TUI parts * modify cmd * recreate stack after modify * add checks to prevent other modifications while modify is applying * modify continue for resuming after resolving conflicts * fix bug with duplicate stack entries after modifying * reuse conflict resolution help msg from rebase * additional confirmation before overwriting stack on remote * fix recreate order of operations Co-authored-by: Copilot <copilot@github.com> * move base commit instead of cherry picking for fold up * check to ensure we aren't left with zero branches * unify and dedupe across view and modify tui * more detailed help instructions Co-authored-by: Copilot <copilot@github.com> * only recommend submit if stack exists on remote Co-authored-by: Copilot <copilot@github.com> * tests for modify tui, apply modifications, submit modifications * refactor submit for regular and pending modifications * rename recover to abort Co-authored-by: Copilot <copilot@github.com> * docs for modify cmd * tui styling updates * updated tui screenshot * addressing review comments * Fix 4 bugs from code review Bug 1: Move RevParseMap error check before using originalRefs. The error from git.RevParseMap() was deferred past iteration of originalRefs, which could panic on a nil map. Bug 2: Differentiate cherry-pick vs rebase conflicts in modify. Cherry-pick conflicts don't save state as 'conflict' phase, so --continue won't work. Now prints --abort-only instructions for cherry-pick conflicts. Bug 3: Unwind now cleans up branches created by renames. After restoring snapshot branches, Unwind deletes renamed branch names that don't belong to the original snapshot. Bug 4: Simplify push message in submit command. Changed from 'Pushing N branches to remote...' to 'Pushing to remote...' since individual branches may fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix 7 nit issues from code review 11: Add named constants for phase strings (PhaseApplying, PhaseConflict, PhasePendingSubmit) in state.go; replace remaining raw literals in state.go CheckStateGuard. 14: Fix bottomLines comment mismatch — listed 3 items but value is 2. 15: Extract magic number 88 to MinWidthForArt constant in header.go. 16: Remove unused stackview import anchor in model.go — the import is used via types.go where BranchNode is embedded. 17: Simplify CheckStackLinearity parent resolution — ActiveBaseBranch already handles skipping merged branches. 18: Fix rename undo matching any rename — add NewName check so only the specific rename being undone is matched. 20: Add TestUndoRename and TestUndoRename_DoesNotAffectOtherRenames to validate rename undo behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make cherry-pick conflicts recoverable via --continue Previously, cherry-pick conflicts during fold-down operations could only be resolved with --abort. Now they save full conflict state (phase, conflict type, fold branch/target, remaining branches) to the state file, enabling recovery via 'gh stack modify --continue'. Changes: - Add ConflictType field to StateFile (rebase or cherry_pick) - Add FoldBranch/FoldTarget fields for cherry-pick context - Add CherryPickContinue to git package (cherry-pick --continue) - Save cherry-pick conflict state in ApplyPlan with remaining branches - ContinueApply handles both rebase and cherry-pick conflicts - Unified conflict messaging in cmd/modify.go (both types show --continue) - Updated test to verify cherry-pick conflict state is saved correctly * Apply suggestions from code review Co-authored-by: Luke Ghenco <lukeghenco@github.com> Co-authored-by: Sameen Karim <skarim@github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Luke Ghenco <lukeghenco@github.com> |